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

HBASE-28509 ScanResumer.resume would perform unnecessary scan when cl… #5817

Merged
merged 5 commits into from
Apr 20, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove terminate in ScanResumer
  • Loading branch information
comnetwork committed Apr 11, 2024
commit 68be28fb09e0105e48241d259c5d6492eaf50503
Original file line number Diff line number Diff line change
@@ -51,18 +51,6 @@ interface ScanResumer {
* effect.
*/
void resume();

/**
* This method is used when {@link ScanController#suspend} had ever been called to get a
* {@link ScanResumer}, but now you stop the scan and do not need more scan results, so you
* invoke this method to tell lower layer that scan is stopped by user and should release
* resources and not initiate new scans any more.
* <p>
* You are free to call it multiple time but only the first call will take effect.
*/
default void terminate() {
resume();
}
}

/**
Original file line number Diff line number Diff line change
@@ -230,7 +230,8 @@ private enum ScanResumerState {
// Notice that, the public methods of this class is supposed to be called by upper layer only, and
// package private methods can only be called within the implementation of
// AsyncScanSingleRegionRpcRetryingCaller.
private final class ScanResumerImpl implements AdvancedScanResultConsumer.ScanResumer {
@InterfaceAudience.Private
final class ScanResumerImpl implements AdvancedScanResultConsumer.ScanResumer {

// INITIALIZED -> SUSPENDED -> RESUMED
// INITIALIZED -> RESUMED
@@ -253,7 +254,10 @@ public void resume() {
doResume(false);
}

@Override
/**
* This method is used when {@link ScanControllerImpl#suspend} had ever been called to get a
* {@link ScanResumerImpl}, but now user stops scan and does not need more scan results.
*/
public void terminate() {
doResume(true);
}
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@
import java.util.ArrayDeque;
import java.util.Queue;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.AsyncScanSingleRegionRpcRetryingCaller.ScanResumerImpl;
import org.apache.hadoop.hbase.client.metrics.ScanMetrics;
import org.apache.hadoop.hbase.util.FutureUtils;
import org.apache.yetus.audience.InterfaceAudience;
@@ -143,14 +144,16 @@ private void resumePrefetch() {
resumer = null;
}

private void closeResumer() {
if (resumer != null) {
if (LOG.isDebugEnabled()) {
LOG.debug(String.format("0x%x", System.identityHashCode(this)) + "close resumer");
}
resumer.terminate();
resumer = null;
private void termianteResumerIfPossible() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? terminateResumerIfPossible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have fixed it.

if (resumer == null) {
return;
}
if (resumer instanceof ScanResumerImpl) {
((ScanResumerImpl) resumer).terminate();
Apache9 marked this conversation as resolved.
Show resolved Hide resolved
} else {
resumePrefetch();
}
resumer = null;
}

@Override
@@ -183,7 +186,7 @@ public synchronized void close() {
closed = true;
queue.clear();
cacheSize = 0;
closeResumer();
termianteResumerIfPossible();
notifyAll();
}