[LIVY-544] Allow interpreterExecutor run in ThreadPool

[WeiWenda]

What changes were proposed in this pull request?

make interpreterExecutor run with newFixedThreadPool rather than newSingleThreadExecutor https://issues.apache.org/jira/browse/LIVY-544

How was this patch tested?

we run a jmeter test of 20 threads each post statement 1/60s, set "spark.scheduler.mode": "FAIR" we find setJobGroup is not thread-safe causing some reflect Error, so wrap it by Session.synchronized

this.synchronized{ setJobGroup(tpe, statementId) }

[codecov-io]

Codecov Report

Merging #135 into master will increase coverage by 0.05%. The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #135      +/-   ##
============================================
+ Coverage     68.38%   68.44%   +0.05%     
  Complexity      891      891              
============================================
  Files           100      100              
  Lines          5596     5597       +1     
  Branches        839      839              
============================================
+ Hits           3827     3831       +4     
+ Misses         1223     1220       -3     
  Partials        546      546

Impacted Files	Coverage Δ	Complexity Δ
.../src/main/scala/org/apache/livy/repl/Session.scala	67.18% <100%> (ø)	37 <0> (ø)	:arrow_down:
rsc/src/main/java/org/apache/livy/rsc/RSCConf.java	85.98% <100%> (+0.13%)	7 <0> (ø)	:arrow_down:
...cala/org/apache/livy/scalaapi/ScalaJobHandle.scala	50% <0%> (-5.89%)	7% <0%> (ø)
...scala/org/apache/livy/repl/PythonInterpreter.scala	48.12% <0%> (+0.62%)	9% <0%> (ø)	:arrow_down:
...ain/scala/org/apache/livy/utils/SparkYarnApp.scala	73.23% <0%> (+0.7%)	33% <0%> (ø)	:arrow_down:
...main/scala/org/apache/livy/server/LivyServer.scala	35.51% <0%> (+1.63%)	9% <0%> (ø)	:arrow_down:

---

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 e2d2189...01a9e0a. Read the comment docs.

[codecov-io]

Codecov Report

Merging #135 into master will decrease coverage by 0.26%. The diff coverage is 46.66%.

@@             Coverage Diff              @@
##             master     #135      +/-   ##
============================================
- Coverage     68.82%   68.56%   -0.27%     
+ Complexity      906      905       -1     
============================================
  Files           100      100              
  Lines          5662     5688      +26     
  Branches        848      856       +8     
============================================
+ Hits           3897     3900       +3     
- Misses         1214     1232      +18     
- Partials        551      556       +5

Impacted Files	Coverage Δ	Complexity Δ
rsc/src/main/java/org/apache/livy/rsc/RSCConf.java	86.11% <100%> (+0.26%)	7 <0> (ø)	:arrow_down:
...in/scala/org/apache/livy/repl/SQLInterpreter.scala	73.33% <100%> (+2.96%)	7 <1> (ø)	:arrow_down:
.../src/main/scala/org/apache/livy/repl/Session.scala	65.32% <30%> (-1.87%)	37 <0> (ø)
...main/java/org/apache/livy/rsc/ContextLauncher.java	64.22% <33.33%> (-1.8%)	14 <0> (+1)
...src/main/scala/org/apache/livy/sessions/Kind.scala	72.72% <50%> (-2.28%)	2 <0> (ø)
...in/java/org/apache/livy/rsc/driver/JobWrapper.java	82.85% <0%> (-5.72%)	8% <0%> (-1%)
...ain/scala/org/apache/livy/utils/SparkYarnApp.scala	76.05% <0%> (-3.53%)	33% <0%> (ø)
...ala/org/apache/livy/scalaapi/LivyScalaClient.scala	83.33% <0%> (-3.34%)	7% <0%> (ø)
...ain/java/org/apache/livy/rsc/driver/RSCDriver.java	77.96% <0%> (-0.85%)	41% <0%> (ø)
...main/scala/org/apache/livy/server/LivyServer.scala	35.96% <0%> (+0.49%)	11% <0%> (ø)	:arrow_down:
... and 1 more

---

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 5767789...936f554. Read the comment docs.

[Tagar]

@WeiWenda thanks for changing to concurrentSQL .

It seems all tests fail with

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

Is this a known issue? cc @vanzin

[WeiWenda]

@WeiWenda thanks for changing to concurrentSQL .

It seems all tests fail with

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

Is this a known issue? cc @vanzin

I have fixed all errors. Can you merge this pr. Thank you.

[Tagar]

@WeiWenda I am not a committer.

Thanks for this great improvement. It would be awesome to have concurrentSQL option in Livy, like we have in bare Zeppelin.

cc @mgaido91 @jerryshao @vanzin @ajbozarth @zjffdu - please help to review.

Thank you.

[ajbozarth]

I see no issues with the code, but since I don't have any experience with the SQL side of Livy I will leave final review and merging to a more knowledgable committer

[Tagar]

Would somebody else from committers be available to review this? cc @jerryshao @vanzin @zjffdu

my 2 cents - It's one of the most exciting Livy improvements among currently open PRs. Since it's backward compatible and disabled by default, it seems fairly safe to add this in.

Thanks everyone.

[jcdauchy]

Hello,

With this feature, would it mean that we can run several statement at the same time on one Livy session ? Regarding of course, that each statement is independant from the other.

Thanks.

[WeiWenda]

Hello,

With this feature, would it mean that we can run several statement at the same time on one Livy session ? Regarding of course, that each statement is independant from the other.

Thanks.

Yes, you can think so.

[jcdauchy]

Hope to find some time to test. Is this the way you use this feature ?

[mtNachiket]

@WeiWenda @mgaido91 When will this be merged as its been almost year and half old ? I have similar use case and this would solve my problem.

[mgaido91]

this is not my main area of expertise, @vanzin and @jerryshao might be better reviewers