add InteractiveSession and SessionManager #2290

penghuo · 2023-10-12T21:43:59Z

Description

add InteractiveSession and SessionManager

Issues Resolved

Phase 3. #2089

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Peng Huo <[email protected]>

codecov · 2023-10-12T22:06:16Z

Codecov Report

Merging #2290 (9693f31) into main (b654e43) will increase coverage by 0.00%.
Report is 2 commits behind head on main.
The diff coverage is 96.87%.

@@            Coverage Diff             @@
##               main    #2290    +/-   ##
==========================================
  Coverage     96.60%   96.61%            
- Complexity     4738     4777    +39     
==========================================
  Files           442      449     +7     
  Lines         12741    12907   +166     
  Branches        876      882     +6     
==========================================
+ Hits          12309    12470   +161     
- Misses          424      429     +5     
  Partials          8        8

Flag	Coverage Δ
sql-engine	`96.61% <96.87%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ql/spark/execution/session/InteractiveSession.java	`100.00% <100.00%> (ø)`
...nsearch/sql/spark/execution/session/SessionId.java	`100.00% <100.00%> (ø)`
...ch/sql/spark/execution/session/SessionManager.java	`100.00% <100.00%> (ø)`
...arch/sql/spark/execution/session/SessionState.java	`100.00% <100.00%> (ø)`
...earch/sql/spark/execution/session/SessionType.java	`100.00% <100.00%> (ø)`
...arch/sql/spark/execution/session/SessionModel.java	`98.48% <98.48%> (ø)`
.../spark/execution/statestore/SessionStateStore.java	`87.50% <87.50%> (ø)`

... and 1 file with indirect coverage changes

vmmusings · 2023-10-13T00:16:41Z

spark/src/main/java/org/opensearch/sql/spark/execution/session/CreateSessionRequest.java

+@Data
+public class CreateSessionRequest {
+  private final StartJobRequest startJobRequest;
+  private final String datasourceName;


Do we require datasourceName in asyncQuery Request model?

datasourceName is write to Session doc in flint.ql.sessions index. it is required for user perform doc level access control.

I am actually asking for this api _plugins/_async_query. I think we need it.
Should we perform datasource authorization in both the calls?

for create session, _plugins/_async_query/sessions, require datasource, address it in separate PR.

we need datasource authZ for both _plugins/_async_query/sessions and _plugins/_async_query API.

vmmusings · 2023-10-13T00:21:03Z

spark/src/main/java/org/opensearch/sql/spark/execution/session/SessionState.java

+public enum SessionState {
+  NOT_STARTED("not_started"),
+  RUNNING("running"),
+  DEAD("dead"),


Whats the difference between DEAD and FAILED?

CLOSE_SESSION event transit state to DEAD.

SPARK_APP FINISH SUCCESS event transit state to DEAD.

SPARK_APP FINISH FAILED event transit state to FAILED

vmmusings · 2023-10-13T00:21:29Z

spark/src/main/java/org/opensearch/sql/spark/execution/session/SessionModel.java

+@Data
+@Builder
+public class SessionModel implements ToXContentObject {
+  public static final String VERSION = "version";


What is the usecase for version here?

In case schema upgrade.

kaituo · 2023-10-13T18:05:30Z

spark/src/main/java/org/opensearch/sql/spark/execution/session/SessionManager.java

+              .sessionStateStore(stateStore)
+              .serverlessClient(emrServerlessClient)
+              .sessionModel(model.get())
+              .build();


Is it ok that the session's createSessionRequest is not set?

done. make sense

createSessionRequest is open() method parameters.

https://github.com/opensearch-project/sql/pull/2290/files#diff-9f0adc022d9f7f7b7cf9d719a10d7dba7b9cd345d989327a5806d270804bdc12

spark/src/main/java/org/opensearch/sql/spark/execution/session/SessionModel.java

Signed-off-by: Peng Huo <[email protected]>

penghuo · 2023-10-13T21:34:00Z

test passed
bwc test failed, sql-2.12 not ready yet.

* add InteractiveSession and SessionManager Signed-off-by: Peng Huo <[email protected]> * address comments Signed-off-by: Peng Huo <[email protected]> --------- Signed-off-by: Peng Huo <[email protected]> (cherry picked from commit f856cb3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add InteractiveSession and SessionManager * address comments --------- (cherry picked from commit f856cb3) Signed-off-by: Peng Huo <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add InteractiveSession and SessionManager * address comments --------- (cherry picked from commit f856cb3) Signed-off-by: Peng Huo <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> (cherry picked from commit 79031a4) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add InteractiveSession and SessionManager * address comments --------- (cherry picked from commit f856cb3) (cherry picked from commit 79031a4) Signed-off-by: Peng Huo <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…#2290) (opensearch-project#2293) (opensearch-project#2315)" This reverts commit 6ac197b.

* Revert "Add more metrics and handle emr exception message (#2422) (#2426)" This reverts commit b57f7cc. * Revert "Block settings in sql query settings API and add more unit tests (#2407) (#2412)" This reverts commit 3024737. * Revert "Added session, statement, emrjob metrics to sql stats api (#2398) (#2400)" This reverts commit 6e17ae6. * Revert "Redefine Drop Index as logical delete (#2386) (#2397)" This reverts commit e939bb6. * Revert "add concurrent limit on datasource and sessions (#2390) (#2395)" This reverts commit deb3ccf. * Revert "Add Flint Index Purging Logic (#2372) (#2389)" This reverts commit dd48b9b. * Revert "Refactoring for tags usage in test files and also added explicit denly list setting. (#2383) (#2385)" This reverts commit 37e010f. * Revert "Enable session by default (#2373) (#2375)" This reverts commit 7d95e4c. * Revert "Create new session if client provided session is invalid (#2368) (#2371)" This reverts commit 5ab7858. * Revert "Add where clause support in create statement (#2366) (#2370)" This reverts commit b620a56. * Revert "create new session if current session not ready (#2363) (#2365)" This reverts commit 5d07281. * Revert "Handle Describe,Refresh and Show Queries Properly (#2357) (#2362)" This reverts commit 16e2f30. * Revert "Add Session limitation (#2354) (#2359)" This reverts commit 0f334f8. * Revert "Bug Fix, support cancel query in running state (#2351) (#2353)" This reverts commit 9a40591. * Revert "Fix bug, using basic instead of basicauth (#2342) (#2355)" This reverts commit e4827a5. * Revert "Add missing tags and MV support (#2336) (#2346)" This reverts commit 8791bb0. * Revert "[Backport 2.x] deprecated job-metadata-index (#2340) (#2343)" This reverts commit bea432c. * Revert "Integration with REPL Spark job (#2327) (#2338)" This reverts commit 58a5ae5. * Revert "Implement patch API for datasources (#2273) (#2329)" This reverts commit 4c151fe. * Revert "Add sessionId parameters for create async query API (#2312) (#2324)" This reverts commit 3d1a376. * Revert "Add Statement (#2294) (#2318) (#2319)" This reverts commit b3c2e94. * Revert "Upgrade json (#2307) (#2314)" This reverts commit 6c65bb4. * Revert "Minor Refactoring (#2308) (#2317)" This reverts commit 051cc4f. * Revert "add InteractiveSession and SessionManager (#2290) (#2293) (#2315)" This reverts commit 6ac197b. --------- Co-authored-by: Vamsi Manohar <[email protected]>

add InteractiveSession and SessionManager

78acb1a

Signed-off-by: Peng Huo <[email protected]>

penghuo requested review from pjfitzgibbons, ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB, rupal-bq, mengweieric, vmmusings, Swiddis, seankao-az, MaxKsyunz, Yury-Fridlyand, anirudha, forestmvey, acarbonetto and GumpacG as code owners October 12, 2023 21:44

penghuo mentioned this pull request Oct 12, 2023

[FEATURE] OpenSearch and EMR-Serverless Integration #2089

Open

46 tasks

penghuo added the backport 2.x label Oct 12, 2023

penghuo self-assigned this Oct 12, 2023

vmmusings reviewed Oct 13, 2023

View reviewed changes

kaituo reviewed Oct 13, 2023

View reviewed changes

dai-chen reviewed Oct 13, 2023

View reviewed changes

spark/src/main/java/org/opensearch/sql/spark/execution/session/SessionModel.java Show resolved Hide resolved

dai-chen previously approved these changes Oct 13, 2023

View reviewed changes

address comments

9693f31

Signed-off-by: Peng Huo <[email protected]>

penghuo dismissed dai-chen’s stale review via 9693f31 October 13, 2023 20:57

dai-chen approved these changes Oct 13, 2023

View reviewed changes

kaituo approved these changes Oct 13, 2023

View reviewed changes

vmmusings approved these changes Oct 13, 2023

View reviewed changes

penghuo merged commit f856cb3 into opensearch-project:main Oct 13, 2023
17 of 21 checks passed

opensearch-trigger-bot bot mentioned this pull request Oct 13, 2023

[Backport 2.x] add InteractiveSession and SessionManager #2293

Merged

vmmusings added a commit to mengweieric/sql that referenced this pull request Nov 13, 2023

Revert "add InteractiveSession and SessionManager (opensearch-project…

e6eac9b

…#2290) (opensearch-project#2293) (opensearch-project#2315)" This reverts commit 6ac197b.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add InteractiveSession and SessionManager #2290

add InteractiveSession and SessionManager #2290

penghuo commented Oct 12, 2023 •

edited

Loading

codecov bot commented Oct 12, 2023 •

edited

Loading

vmmusings Oct 13, 2023

penghuo Oct 13, 2023

vmmusings Oct 13, 2023

penghuo Oct 13, 2023

vmmusings Oct 13, 2023

penghuo Oct 13, 2023

vmmusings Oct 13, 2023 •

edited

Loading

penghuo Oct 13, 2023

kaituo Oct 13, 2023

penghuo Oct 13, 2023 •

edited

Loading

penghuo commented Oct 13, 2023

add InteractiveSession and SessionManager #2290

add InteractiveSession and SessionManager #2290

Conversation

penghuo commented Oct 12, 2023 • edited Loading

Description

Issues Resolved

Check List

codecov bot commented Oct 12, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmmusings Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penghuo Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

penghuo commented Oct 13, 2023

penghuo commented Oct 12, 2023 •

edited

Loading

codecov bot commented Oct 12, 2023 •

edited

Loading

vmmusings Oct 13, 2023 •

edited

Loading

penghuo Oct 13, 2023 •

edited

Loading