-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-28396: Quota throttling can cause a leak of scanners #6055
base: master
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
I don't think the build failure is related to my change |
Just ignore the Hadoop 3.4.0 Hadoop check failure. The branch itself is broken. |
I'm working on adding a test at the moment that will sanity check the scanner counts |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@@ -216,7 +216,7 @@ public String getName() { | |||
} | |||
|
|||
/** This class tracks a single Lease. */ | |||
static class Lease implements Delayed { | |||
protected static class Lease implements Delayed { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if there's a better way to do this, but I needed to expose this to implement this method to track the leases created
public void itIncreasesScannerCount() throws Exception { | ||
long beforeScan = countLeases(); | ||
ResultScanner scanner = TABLE.getScanner(new Scan()); | ||
Thread.sleep(1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to wait until the scan and lease are created server-side. Otherwise, our lease counting will not reflect this new scan that was created
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
We've noticed memory issues during intense periods of quota throttling. This happens because new scanners are created prior to the acquisition of the throttle, and aren't cleaned up correctly in the case acquiring the throttle throws an exception. This change creates a new region scanner only if the quota was acquired successfully to avoid scanner leaks.