SwaraliJoshi commented on code in PR #8357:
URL: https://github.com/apache/hbase/pull/8357#discussion_r3584730152
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRollbackSCP.java:
##########
@@ -75,12 +85,45 @@ public class TestRollbackSCP {
private static final AtomicBoolean INJECTED = new AtomicBoolean(false);
+ // HBASE-29555: guards the in-place procedure-executor reload against region
servers concurrently
+ // reporting region state transitions. See
restartMasterProcedureExecutorLikeFailover(). Fair so
+ // that, while the reload is waiting for / holding the write lock, new
reports are rejected rather
+ // than barging in. A region state report takes the read lock; the reload
takes the write lock.
+ private static final ReentrantReadWriteLock RELOAD_LOCK = new
ReentrantReadWriteLock(true);
Review Comment:
Fairness is there to prevent the reload thread (the writer) from being
starved by a steady stream of region-state reports (the readers). The report
path uses the timed tryLock(0, …), which honors the fairness policy: with a
fair lock, once the reload is queued waiting for the write lock, new tryLock
attempts return false and the report is rejected (PleaseHoldException, RS
retries), so the writer acquires promptly. With a non-fair lock, readers
"barge" ahead of the waiting writer, so under continuous reports the reload
could be delayed.
Fairness prevents reload starvation under report load, happy to drop it if
you think that's over-engineering.
--
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]
