Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[KYUUBI #5952] Disconnect connections without running operations after engine maxlife time graceful period #5950

Closed
wants to merge 1 commit into from

Conversation

turboFei
Copy link
Member

@turboFei turboFei commented Jan 5, 2024

🔍 Description

Issue References 🔗

We found that, some kyuubi connections(maybe managed by jdbc connection pool likes hikari) always keep alive, and the engine can not be terminated after exceeds the max life time.

So, In this pr, I introduce a graceful period after spark engine max life time, after the graceful period, the connections without running operations will be disconnected forcibly.

Close #5952

Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklist 📝

Be nice. Be informative.

@turboFei turboFei self-assigned this Jan 5, 2024
@turboFei turboFei added this to the v1.8.1 milestone Jan 5, 2024
@turboFei turboFei requested review from cxzl25 and pan3793 January 5, 2024 17:21
@codecov-commenter
Copy link

codecov-commenter commented Jan 5, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (87ed400) 61.20% compared to head (bd18a38) 61.21%.
Report is 1 commits behind head on master.

❗ Current head bd18a38 differs from pull request most recent head 0352dd1. Consider uploading reports for the commit 0352dd1 to get more accurate results

Files Patch % Lines
...rg/apache/kyuubi/engine/spark/SparkSQLEngine.scala 69.23% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5950      +/-   ##
============================================
+ Coverage     61.20%   61.21%   +0.01%     
  Complexity       23       23              
============================================
  Files           622      622              
  Lines         36897    36899       +2     
  Branches       5016     5017       +1     
============================================
+ Hits          22582    22589       +7     
+ Misses        11875    11868       -7     
- Partials       2440     2442       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@turboFei turboFei marked this pull request as draft January 6, 2024 05:32
@github-actions github-actions bot added kind:documentation Documentation is a feature! module:common labels Jan 6, 2024
@turboFei turboFei changed the title [KYUUBI #2250][FOLLOWUP] Terminating spark engine if life time exceeds and no running operations [KYUUBI #2250][FOLLOWUP] Disconnect connections without running operations after engine maxlife time graceful period Jan 6, 2024
@turboFei turboFei marked this pull request as ready for review January 6, 2024 06:21
@turboFei turboFei force-pushed the close_on_nooperation branch 2 times, most recently from fa2fe08 to fe46552 Compare January 7, 2024 15:07
@cxzl25
Copy link
Contributor

cxzl25 commented Jan 8, 2024

#2250 has been released for a while, I recommend using a new issue id to track the issue.

@turboFei turboFei changed the title [KYUUBI #2250][FOLLOWUP] Disconnect connections without running operations after engine maxlife time graceful period [KYUUBI #5952] Disconnect connections without running operations after engine maxlife time graceful period Jan 8, 2024
@turboFei
Copy link
Member Author

turboFei commented Jan 8, 2024

Not sure why EtcdConnectionLevelSparkEngineSuite always fail, cc @hddong is there any special for ETCD?

@wForget
Copy link
Member

wForget commented Jan 14, 2024

We found that, some kyuubi connections(maybe managed by jdbc connection pool likes hikari) always keep alive, and the engine can not be terminated after exceeds the max life time.

Shall we reject new operations after the engine has stopped gracefully? That way the connection will be closed after session idle timeout.

@turboFei
Copy link
Member Author

Shall we reject new operations after the engine has stopped gracefully? That way the connection will be closed after session idle timeout.

It is difficult to implement in code.

Some rpc does not create operation but will update the session last access time with withAcquireRelease.

@pan3793
Copy link
Member

pan3793 commented Jan 15, 2024

@turboFei the jetcd upgrade involves gRPC upgrading, I have done it on master, please revert your latest commit and rebase master to see if helps

ut

ut

revert

save

save

debug

Revert "debug"

This reverts commit fe46552.

retry
@turboFei
Copy link
Member Author

raised #6020

@turboFei turboFei closed this Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Disconnect connections without running operations after engine maxlife time graceful period
6 participants