-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-24694 Support flush a single column family of table #2179
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
Conversation
hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/FlushTableSubprocedure.java
Outdated
Show resolved
Hide resolved
| void flush(TableName tableName) throws IOException; | ||
|
|
||
| /** | ||
| * Flush a table. Synchronous operation. |
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 should explain better how this overloaded version differs from flush(TableName tableName).
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.
Will fix.
| CompletableFuture<Void> flush(TableName tableName); | ||
|
|
||
| /** | ||
| * Flush a table. |
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 should explain better how this overloaded version differs from flush(TableName tableName).
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.
Will fix.
| try { | ||
| LOG.debug("Flush region " + region.toString() + " started..."); | ||
| region.flush(true); | ||
| region.flushcache(families, false, FlushLifeCycleTracker.DUMMY); |
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.
Can this affect the other versions that don't specify any families, like flush(TableName tableName)? Because region.flush(true), internally, loads all families before delegating to flushcache(List<byte[]> families, boolean writeFlushRequestWalMarker, FlushLifeCycleTracker tracker), but it doesn't seem we are doing this now.
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.
You are right, it indeed affect, will fix.
Thanks.
...ain/java/org/apache/hadoop/hbase/procedure/flush/RegionServerFlushTableProcedureManager.java
Show resolved
Hide resolved
|
🎊 +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. |
wchevreuil
left a comment
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.
lgtm +1, just small nits around javadocs and help output text.
| void flush(TableName tableName) throws IOException; | ||
|
|
||
| /** | ||
| * Flush a table with specified column family. Synchronous operation. |
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.
Let's not leave any room for confusion: Flush the specified column family stores on all regions of the passed table. This runs as a synchronous operation.
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.
Will fix later.
Thanks.
| CompletableFuture<Void> flush(TableName tableName); | ||
|
|
||
| /** | ||
| * Flush a table with specified column family. |
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.
Similar to above.
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.
Will fix later.
Thanks.
| flush an individual region or a region server name whose format | ||
| is 'host,port,startcode', to flush all its regions. | ||
| You can also flush a single column family within a region. | ||
| You can also flush a single column family within a table or region. |
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.
You can also flush a single column family for all regions within a table, or for an specific region only.
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.
Will fix later.
Thanks.
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Test failure on jdk11 seems unrelated to this PR. |
|
Thanks for you time. @wchevreuil |
No description provided.