HBASE-28828 Should check if table exists firstly when executing ExpiredMobFileCleaner#6231
Closed
guluo2016 wants to merge 2 commits into
Closed
HBASE-28828 Should check if table exists firstly when executing ExpiredMobFileCleaner#6231guluo2016 wants to merge 2 commits into
guluo2016 wants to merge 2 commits into
Conversation
guluo2016
commented
Sep 9, 2024
| ColumnFamilyDescriptor family = htd.getColumnFamily(Bytes.toBytes(familyName)); | ||
| if (family == null || !family.isMobEnabled()) { | ||
| if (family == null) { | ||
| throw new IOException("Column family " + familyName + " doesn't exist"); |
Member
Author
There was a problem hiding this comment.
When column family does not exist, we provide accurate information in here
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
chandrasekhar-188k
approved these changes
Sep 14, 2024
Apache9
reviewed
Sep 14, 2024
| if (!admin.tableExists(tn)) { | ||
| throw new IOException("The table " + tableName + " doesn't exist"); | ||
| } | ||
| TableDescriptor htd = admin.getDescriptor(tn); |
Contributor
There was a problem hiding this comment.
I think getDescriptor will lead to a TableNotFoundException, which is the same here? TableNotFOundException is an IOException?
Member
Author
There was a problem hiding this comment.
Yes, admin.getDescriptor would throw TableNotFoundException when table doesn't exist.
In here, What I mean is that we provide clear and concise information about that table doesn't exist, as following, maybe this would be better?
./bin/hbase org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner t01 in
2024-09-15T14:02:26,263 WARN [main {}] util.NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
2024-09-15T14:02:26,940 INFO [RPCClient-NioEventLoopGroup-1-2 {}] Configuration.deprecation: hbase.client.pause.cqtbe is deprecated. Instead, use hbase.client.pause.server.overloaded
2024-09-15T14:02:27,040 INFO [main {}] client.AsyncConnectionImpl: Connection has been closed by main.
Exception in thread "main" java.io.IOException: The table t01 doesn't exist
at org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner.run(ExpiredMobFileCleaner.java:99)
at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:82)
at org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner.main(ExpiredMobFileCleaner.java:75)
2024-09-15T14:02:27,050 INFO [Registry-endpoints-refresh-end-points {}] client.RegistryEndpointsRefresher: Registry end points refresher loop exited.
If we donot do this, when table does not exist, we will get the follow message.
Exception in thread "main" org.apache.hadoop.hbase.TableNotFoundException: t04_mob01
at java.base/java.lang.Thread.getStackTrace(Thread.java:1610)
at org.apache.hadoop.hbase.util.FutureUtils.setStackTrace(FutureUtils.java:144)
at org.apache.hadoop.hbase.util.FutureUtils.rethrow(FutureUtils.java:163)
at org.apache.hadoop.hbase.util.FutureUtils.get(FutureUtils.java:186)
at org.apache.hadoop.hbase.client.AdminOverAsyncAdmin.getDescriptor(AdminOverAsyncAdmin.java:172)
at org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner.run(ExpiredMobFileCleaner.java:99)
at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:82)
at org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner.main(ExpiredMobFileCleaner.java:75)
at -------Future.get-------(Unknown Source)
at org.apache.hadoop.hbase.client.RawAsyncHBaseAdmin.lambda$getDescriptor$24(RawAsyncHBaseAdmin.java:654)
at org.apache.hadoop.hbase.util.FutureUtils.lambda$addListener$0(FutureUtils.java:71)
at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863)
at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:841)
at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147)
at org.apache.hadoop.hbase.client.AsyncMasterRequestRpcRetryingCaller.lambda$doCall$4(AsyncMasterRequestRpcRetryingCaller.java:80)
at org.apache.hadoop.hbase.util.FutureUtils.lambda$addListener$0(FutureUtils.java:71)
at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863)
at java.base/java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:841)
at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2147)
at org.apache.hadoop.hbase.client.RawAsyncHBaseAdmin$1.run(RawAsyncHBaseAdmin.java:467)
at org.apache.hbase.thirdparty.com.google.protobuf.RpcUtil$1.run(RpcUtil.java:56)
Contributor
There was a problem hiding this comment.
I'm not very familiar with mob, why the table name changed to t04_mob01 from t01?
Member
Author
There was a problem hiding this comment.
Sorry about that, I didnot change table name
I just used 2 examples in here.
The first example, i specfied a non_existing table named t01
./bin/hbase org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner t01 inThe second example , i specfied a non_existing table named t04_mob
./bin/hbase org.apache.hadoop.hbase.mob.ExpiredMobFileCleaner t04_mob inIn addition， to execute ExpiredMobFileCleaner , two parameters need to be specified, table name and column family
Contributor
There was a problem hiding this comment.
So the table name will be changed from t04_mob to t04_mob01? This is the problem here? Where do we do this convertion?
Contributor
There was a problem hiding this comment.
Then back to my very first comment here, the old logic seems enough to indicate that the table does not exist? We do not need an extra table exists check?
Member
Author
There was a problem hiding this comment.
Yes, admin.getDescriptor(tn) would throw TNFE when the table does not exist.
As I mentioned before，this commit is order to optimize this exception messge, providing clear and concise information
Contributor
There was a problem hiding this comment.
I do not think java.io.IOException: The table non_existing_table doesn't exist is more clear than org.apache.hadoop.hbase.TableNotFoundException: non_existing_table...
They are just the same...
Member
Author
There was a problem hiding this comment.
They are just the same...
Yes, let me close this PR later.
Thank you very much for your patient review.
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details see: HBASE-28828