rishabhdaim commented on code in PR #1288:
URL: https://github.com/apache/jackrabbit-oak/pull/1288#discussion_r1471678539
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/ClusterNodeInfo.java:
##########
@@ -229,13 +229,14 @@ static RecoverLockState fromString(String state) {
/** OAK-3399 : max number of times we're doing a 1sec retry loop just
before declaring lease failure **/
private static final int MAX_RETRY_SLEEPS_BEFORE_LEASE_FAILURE = 5;
- /** OAK-10281 : seconds to delay a recovery after a lease timeout */
- private static final int DEFAULT_RECOVERY_DELAY_SECS =
SystemPropertySupplier.create("oak.documentMK.recoveryDelaySecs", 0)
- .loggingTo(LOG).validateWith(value -> value >= 0)
- .formatSetMessage((name, value) -> String.format("recovery delay
set to (secs): %ss (using system property %s)", name, value)).get();
- private static final long DEFAULT_RECOVERY_DELAY_MILLIS = 1000L *
(long)DEFAULT_RECOVERY_DELAY_SECS;
+ /** OAK-10281 : default millis to delay a recovery after a lease timeout */
+ static final long DEFAULT_RECOVERY_DELAY_MILLIS = 0;
- // kept non-final for testing purpose
+ /**
+ * Actual millis to delay a recovery after a lease timeout.
+ * <p>
+ * Initialized by DocumentNodeStore constructor.
+ */
static long recoveryDelayMillis = DEFAULT_RECOVERY_DELAY_MILLIS;
Review Comment:
To me it seems to be an anti-pattern or hacky way of implementing this
change.
IMO our approach shouldn't be decided by no. of changes rather than correct
way of implementing it.
But in this case if you think the no. of changes are too much of an effort
vs the gain that we get from them, then may be we can go with static field
approach as well. I will leave that upto you to decide.
--
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]
