wchevreuil commented on code in PR #7084:
URL: https://github.com/apache/hbase/pull/7084#discussion_r2138089524
##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureSuspendedException.java:
##########
@@ -35,5 +38,21 @@ public ProcedureSuspendedException() {
*/
public ProcedureSuspendedException(String s) {
super(s);
+ shouldForceLockRelease = false;
+ }
+
+ /**
+ * Constructor
+ * @param s message
+ * @param shouldForceLockRelease if true, the procedure will release the
lock regardless of
+ * {@link Procedure#holdLock(Object)}
+ */
+ public ProcedureSuspendedException(String s, boolean shouldForceLockRelease)
{
+ super(s);
+ this.shouldForceLockRelease = shouldForceLockRelease;
+ }
+
+ public boolean shouldForceLockRelease() {
Review Comment:
>I think the comment still holds. We want to avoid
enabling/disabling/modifying/deleting a table during a snapshot.
We may want to avoid, but now we are allowing a case where we suspend and
release the lock.
> Alternatively, maybe a less confusing option would be to return a
non-static value for `holdLock` in `SnapshotProcedure`
>
That's what I meant by "modify the SnapshotProcedure.holdLock() behaviour".
You are already modifying SnapshotProcedure behaviour to cause a release lock
suspension via a flag in the ProcedureSuspendedException. We could keep this
"flag" internally in SnapshotProcedure and use it to define what
SnapshotProcedure.holdLock() returns.
> This forces you to manage additional state at each stage though, which I
why I opted for this implementation which I thought was safer
True. Yet, it seems more intuitive to me. But that's a personal opinion,
just wanted to make sure it was also considered. Up to you to decide on the
"final" solution. If going with the original, just please amend the comment in
holdLock to explain the scenario where we release the lock.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
