-
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-28150 CreateTableProcedure and DeleteTableProcedure should slee… #5502
Conversation
🎊 +1 overall
This message was automatically generated. |
long sleepTime = retryCounter.getBackoffTimeAndIncrementAttempts(); | ||
LOG.warn("Retriable error trying to delete table=" + getTableName() + " state=" + state | ||
+ ", Will sleep " + sleepTime / 1000 + " secs and try again later", e); | ||
Threads.sleep(sleepTime); |
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.
Better not sleep directly here as it will block e PEWorker. You can call suspend method from the parent Procedure class to suspend the procedure for a while.
And we also need to reset the retryCounter if there is no problem after retrying.
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.
thanks for reviwing
- i have some questions...
you suggest call suspend method from the parent Procedure?
but CreateTableProcedure did not have a parent proc, or you mean we need add a suspent method to
AbstractStateMachineTableProcedure?
For the old logic, TRSP could suspent, but CreateTableProcedure could not..
2.yeah, we need reset the retryCounter...
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.
The suspend method is in the Procedure class, which is the parent class for all procedures.
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.
Check out #5534 for a recent example of doing this pausing
Any updates here? |
Ping @chaijunjie0101 |
i am so sorry, i forget it... i will fix it as soon as possible. |
db20f6a
to
da1cc82
Compare
@bbeaudreault thanks for suggestting |
💔 -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. |
long backoff = retryCounter.getBackoffTimeAndIncrementAttempts(); | ||
LOG.warn("Retriable error trying to create table={},state={},suspend {}secs.", | ||
getTableName(), state, backoff / 1000, e); | ||
throw suspend(Math.toIntExact(backoff), true); | ||
} | ||
} | ||
return Flow.HAS_MORE_STATE; |
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 need to reset the retryCounter to null here. The reason for resetting is that, if we have successfully finished one step, we should reset the retryCounter.
long backoff = retryCounter.getBackoffTimeAndIncrementAttempts(); | ||
LOG.warn("Retriable error trying to delete table={},state={},suspend {}secs.", | ||
getTableName(), state, backoff / 1000, e); | ||
throw suspend(Math.toIntExact(backoff), true); | ||
} | ||
} | ||
return Flow.HAS_MORE_STATE; |
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.
Ditto.
…p a while before retrying
da1cc82
to
8735bb8
Compare
🎊 +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. |
…p a while before retrying (#5502) Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit aaeef2d)
…p a while before retrying (#5502) Signed-off-by: Duo Zhang <[email protected]>
…p a while before retrying (#5502) Signed-off-by: Duo Zhang <[email protected]> (cherry picked from commit 5a404c4)
…p a while before retrying (apache#5502) Signed-off-by: Duo Zhang <[email protected]>
…uld sleep a while before retrying (apache#5502)" This reverts commit aaeef2d.
…p a while before retrying (apache#5502) Signed-off-by: Duo Zhang <[email protected]>
…p a while before retrying