[
https://issues.apache.org/jira/browse/CASSANDRA-21072?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18057018#comment-18057018
]
Arnav Chakraborty edited comment on CASSANDRA-21072 at 2/7/26 8:07 AM:
-----------------------------------------------------------------------
Hi [~brandon.williams] ,
Thanks for the response! I dug deeper into the code and ran the existing
tests to understand the current behavior, and I have a follow-up question.
Looking at [ReadCommand.java (line
684-688)|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/ReadCommand.java#L687],
TOMBSTONE_FAIL is already being set in MessageParams before throwing the
exception when trackWarnings is true:
{code:java}
if (trackWarnings)
{
MessageParams.remove(ParamType.TOMBSTONE_WARNING);
MessageParams.add(ParamType.TOMBSTONE_FAIL, tombstones);
}
throw new TombstoneOverwhelmingException(...);{code}
I also ran TombstoneCountWarningTest.failThresholdSinglePartition() and
confirmed that the coordinator does receive READ_TOO_MANY_TOMBSTONES through
the WarningContext → ReadCallback.onFailure() path when warnings tracking is
enabled.
The test asserts this explicitly:
{code:java}
assertThat(e.getFailuresMap())
.containsValue(RequestFailureReason.READ_TOO_MANY_TOMBSTONES.code){code}
So it seems like the MessageParams approach is already in place and working
end-to-end for TombstoneOverwhelmingException. Given that, could you clarify
what specific change you're proposing? Some possibilities I see:
1. Is the issue that the exception is still being swallowed on the replica
side (logged + empty response) rather than propagated as a proper failure via
MessagingService.respondWithFailure()? Even though the coordinator handles it
correctly via the params side-channel, the mechanism is fragile/inconsistent
with the non-tracking path.
2. Or is there a specific edge case where the MessageParams aren't set
correctly that I'm missing?
3. Or is the goal to remove the RejectException catch block in
ReadCommandVerbHandler entirely and let all RejectException subclasses
propagate naturally (since InboundSink would handle them), now that
MessageParams are being set
regardless?
Want to make sure I understand the intended fix before putting up a patch.
Thanks!
was (Author: JIRAUSER312244):
Hi,
Thanks for the response! I dug deeper into the code and ran the existing
tests to understand the current behavior, and I have a follow-up question.
Looking at [ReadCommand.java (line
684-688)|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/ReadCommand.java#L687],
TOMBSTONE_FAIL is already being set in MessageParams before throwing the
exception when trackWarnings is true:
{code:java}
if (trackWarnings)
{
MessageParams.remove(ParamType.TOMBSTONE_WARNING);
MessageParams.add(ParamType.TOMBSTONE_FAIL, tombstones);
}
throw new TombstoneOverwhelmingException(...);{code}
I also ran TombstoneCountWarningTest.failThresholdSinglePartition() and
confirmed that the coordinator does receive READ_TOO_MANY_TOMBSTONES through
the WarningContext → ReadCallback.onFailure() path when warnings tracking is
enabled.
The test asserts this explicitly:
{code:java}
assertThat(e.getFailuresMap())
.containsValue(RequestFailureReason.READ_TOO_MANY_TOMBSTONES.code){code}
So it seems like the MessageParams approach is already in place and working
end-to-end for TombstoneOverwhelmingException. Given that, could you clarify
what specific change you're proposing? Some possibilities I see:
1. Is the issue that the exception is still being swallowed on the replica
side (logged + empty response) rather than propagated as a proper failure via
MessagingService.respondWithFailure()? Even though the coordinator handles it
correctly via the params side-channel, the mechanism is fragile/inconsistent
with the non-tracking path.
2. Or is there a specific edge case where the MessageParams aren't set
correctly that I'm missing?
3. Or is the goal to remove the RejectException catch block in
ReadCommandVerbHandler entirely and let all RejectException subclasses
propagate naturally (since InboundSink would handle them), now that
MessageParams are being set
regardless?
Want to make sure I understand the intended fix before putting up a patch.
Thanks!
> TombstoneOverwhelmingException gets swallowed when tracking warnings
> --------------------------------------------------------------------
>
> Key: CASSANDRA-21072
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21072
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Feature/Guardrails
> Reporter: Brandon Williams
> Assignee: Arnav Chakraborty
> Priority: Normal
> Fix For: 5.0.x, 6.x
>
>
> ReadCommandVerbHandler is catching RejectException and returning an empty
> response instead of propagating the failure when warnings tracking is
> enabled, see
> https://github.com/datastax/cassandra/commit/f1223ec9fd9c724f00acbe338e053f3533617c8a
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
