FrankChen021 commented on code in PR #19501:
URL: https://github.com/apache/druid/pull/19501#discussion_r3312282613
##########
server/src/main/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResource.java:
##########
@@ -60,9 +60,16 @@ public CoordinatorCompactionConfigsResource(
@Produces(MediaType.APPLICATION_JSON)
public Response getCompactionConfig()
{
- return ServletResourceUtils.buildReadResponse(
+ final Response baseResponse = ServletResourceUtils.buildReadResponse(
configManager::getCurrentCompactionConfig
);
+ if (baseResponse.getStatus() != Response.Status.OK.getStatusCode()) {
+ return baseResponse;
+ }
+ return DynamicConfigEtagHelper.withEtag(
Review Comment:
[P1] ETag can describe a different config than the response body
The ETagged GET path builds the response entity from
`getCurrentCompactionConfig()` and then separately reads
`getCurrentCompactionConfigBytes()` for the ETag. If another config update
lands between those two reads, the response can contain the old config body
with the new stored bytes' ETag. A client can then edit that stale body and
submit it with `If-Match` using the fresh ETag, so the precondition passes and
the unseen concurrent update is overwritten. The same body/bytes split appears
in the other ETagged dynamic-config GET handlers; these should return an atomic
bytes+decoded-config snapshot, or derive both body and ETag from the same byte
array.
--
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]
