[LIVY-239] Generate session IDs in SessionStore, not in SessionManager

[meisam]

What changes were proposed in this pull request?

Move the logic to create session IDs from SessionManager to SessionStore

Why this change is needed?

Generating session IDs at session stores that they are unique across all Livy instances that connect to a session store. Right now only one Livy instance connects to the session store, but to implement multimode HA, generating session IDs in session store prevents inconsistent/colliding values.

How was this patch tested?

• Unit tests are added.
• This patch is working in production at PayPal for about a year now.

JIRA ticket: https://issues.apache.org/jira/browse/LIVY-239

[meisam]

sudo -E apt-get ...

failed on one of the travis instances. Closing this PR and opening it again to rebuild.

[codecov-io]

Codecov Report

Merging #103 into master will decrease coverage by 0.11%. The diff coverage is 61.53%.

@@             Coverage Diff              @@
##             master     #103      +/-   ##
============================================
- Coverage     68.79%   68.67%   -0.12%     
  Complexity      905      905              
============================================
  Files           100      100              
  Lines          5650     5651       +1     
  Branches        846      845       -1     
============================================
- Hits           3887     3881       -6     
- Misses         1211     1219       +8     
+ Partials        552      551       -1

Impacted Files	Coverage Δ	Complexity Δ
...a/org/apache/livy/server/recovery/StateStore.scala	83.33% <ø> (ø)	1 <0> (ø)	:arrow_down:
...che/livy/server/recovery/ZooKeeperStateStore.scala	70.83% <0%> (-8.24%)	17 <0> (ø)
...he/livy/server/recovery/FileSystemStateStore.scala	65.3% <100%> (+2.26%)	12 <1> (+1)	:arrow_up:
...org/apache/livy/server/recovery/SessionStore.scala	82.35% <100%> (+2.35%)	9 <2> (-1)	:arrow_down:
...che/livy/server/recovery/BlackholeStateStore.scala	100% <100%> (ø)	7 <2> (+2)	:arrow_up:
...cala/org/apache/livy/sessions/SessionManager.scala	81.92% <100%> (-0.1%)	23 <1> (-1)
...cala/org/apache/livy/scalaapi/ScalaJobHandle.scala	52.94% <0%> (-2.95%)	7% <0%> (ø)
...c/main/scala/org/apache/livy/repl/ReplDriver.scala	30.76% <0%> (-2.57%)	7% <0%> (ø)
...c/src/main/java/org/apache/livy/rsc/RSCClient.java	73.91% <0%> (-1.25%)	20% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 80daade...3baf264. Read the comment docs.

[alex-the-man]

Can we add some migration code so user can seamlessly upgrade from older Livy? Thanks!

[meisam]

Can we add some migration code so user can seamlessly upgrade from older Livy? Thanks!

Should the migration code be part of Livy? Or should be a separate script that users run once?

Also should we change the version in zookeeper path from /v1/... to /v2/...?

[alex-the-man]

Can we add some migration code so user can seamlessly upgrade from older Livy? Thanks!

Should the migration code be part of Livy? Or should be a separate script that users run once?

In this case it seems simple enough to be a part of the startup code.

Also should we change the version in zookeeper path from /v1/... to /v2/...?

I think you are right, we should.