FrankChen021 commented on code in PR #19501:
URL: https://github.com/apache/druid/pull/19501#discussion_r3317741176
##########
processing/src/main/java/org/apache/druid/common/config/JacksonConfigManager.java:
##########
@@ -127,6 +127,66 @@ public <T> SetResult set(
return configManager.set(key, configSerde, oldValue, newValue);
}
+ @Nullable
+ public byte[] getCurrentBytes(String key)
+ {
+ return configManager.getCurrentBytes(key);
+ }
+
+ public boolean isCompareAndSwapEnabled()
+ {
+ return configManager.isCompareAndSwapEnabled();
+ }
+
+ /**
+ * 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 conditional
+ * writes require {@code druid.manager.config.enableCompareAndSwap} to be
+ * true (the default). With CAS disabled, {@code If-Match} writes fail as a
+ * precondition failure instead of silently degrading to last-writer-wins.
+ */
+ 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);
+ }
+ if (!configManager.isCompareAndSwapEnabled()) {
+ return SetResult.preconditionFailed(
+ new IllegalStateException(
+ "If-Match requires druid.manager.config.enableCompareAndSwap to
be enabled for key[" + key + "]"
+ )
+ );
+ }
+ final byte[] currentBytes = configManager.getCurrentBytes(key);
Review Comment:
[P1] Build conditional partial updates from the CAS snapshot
setIfMatch reads the bytes used for If-Match validation and CAS after
callers have already built newValue. That is safe for full replacements, but
the new Coordinator/Broker/K8s partial-update endpoints merge request fields
with a watched config object before calling this method. If the watcher
advances between that merge and this read, an If-Match for snapshot B can pass
and CAS B -> newValue(A + patch), dropping fields introduced in B while
returning success. Conditional partial writes need a transform-style path that
reads bytes, validates the ETag, deserializes those same bytes, applies the
patch, and CASes the same byte snapshot.
--
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]
