jtuglu1 commented on code in PR #19501:
URL: https://github.com/apache/druid/pull/19501#discussion_r3290501073
##########
processing/src/main/java/org/apache/druid/common/config/JacksonConfigManager.java:
##########
@@ -127,6 +127,54 @@ public <T> SetResult set(
return configManager.set(key, configSerde, oldValue, newValue);
}
+ @Nullable
+ public byte[] getCurrentBytes(String key)
+ {
+ return configManager.getCurrentBytes(key);
+ }
+
+ /**
+ * Set the config, optionally guarded by an {@code If-Match}-style
+ * precondition. When {@code ifMatchEtag} is {@code null}, behaves like
+ * {@link #set(String, Object, AuditInfo)}. Otherwise the write only commits
+ * if the currently stored bytes hash to {@code ifMatchEtag}; on mismatch the
+ * result reports {@link SetResult#isPreconditionFailed()
preconditionFailed}.
+ *
+ * <p>The precondition is enforced via metadata-store CAS, so it only holds
+ * when {@code druid.manager.config.enableCompareAndSwap} is true (the
+ * default). With CAS disabled, the {@code If-Match} check degrades to a
+ * best-effort TOCTOU read and concurrent writers may still race past it.
+ */
+ public <T> SetResult setIfMatch(
+ String key,
+ @Nullable String ifMatchEtag,
+ T newValue,
+ AuditInfo auditInfo
+ )
+ {
+ if (newValue == null) {
+ return SetResult.failure(new IllegalArgumentException("input obj is
null"));
+ }
+ if (ifMatchEtag == null) {
+ return set(key, newValue, auditInfo);
+ }
+ final byte[] currentBytes = configManager.getCurrentBytes(key);
+ if (!ConfigEtag.matches(ifMatchEtag, currentBytes)) {
+ return SetResult.preconditionFailed(
+ new IllegalStateException("If-Match precondition failed for key[" +
key + "]")
+ );
+ }
+ final SetResult result = set(key, currentBytes, newValue, auditInfo);
Review Comment:
Yeah – this is minor IMO. I don't think anyone is running with
`enableCompareAndSwap=false`. If they are, I would expect they don't require
any sort of correctness under concurrency. I suppose we can throw an exception
if an ETag is provided but `enableCompareAndSwap=false`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
