Skip to content

CNDB-16583: Add new param to failure callback messages - verb name of the incoming message.#2244

Merged
MSpryszynski merged 2 commits intomainfrom
cndb-16583
Apr 9, 2026
Merged

CNDB-16583: Add new param to failure callback messages - verb name of the incoming message.#2244
MSpryszynski merged 2 commits intomainfrom
cndb-16583

Conversation

@MSpryszynski
Copy link
Copy Markdown

@MSpryszynski MSpryszynski commented Feb 25, 2026

What is the issue

We want to use the messaging filter to collect metrics for failure responses coming from writer instances. To properly handle different types of failed requests (Read/Write), we need a way to distinguish between callback responses.

Fixes https://github.com/riptano/cndb/issues/16583

What does this PR fix and why was it fixed

This PR adds a new parameter to failure callback messages—the verb name of the incoming request. This parameter allows us to differentiate failure responses based on the origin of the request.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 25, 2026

Checklist before you submit for review

  • This PR adheres to the Definition of Done
  • Make sure there is a PR in the CNDB project updating the Converged Cassandra version
  • Use NoSpamLogger for log lines that may appear frequently in the logs
  • Verify test results on Butler
  • Test coverage for new/modified code is > 80%
  • Proper code formatting
  • Proper title for each commit staring with the project-issue number, like CNDB-1234
  • Each commit has a meaningful description
  • Each commit is not very long and contains related changes
  • Renames, moves and reformatting are in distinct commits
  • All new files should contain the DataStax copyright header instead of the Apache License one

@niksajakovljevic niksajakovljevic self-requested a review March 3, 2026 08:54
*/
TRACE_KEYSPACE (8, "TraceKeyspace", StringSerializer.serializer),
/**
* Failure callback messages contain verb name of the incoming message.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Failure response messages... might sounds more clear?

Copy link
Copy Markdown
Collaborator

@niksajakovljevic niksajakovljevic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can you please inspect the test failures (for some reason I can't access Butler atm).

public Message<RequestFailureReason> failureResponse(RequestFailureReason reason)
{
return failureResponse(id(), expiresAtNanos(), reason);
return failureResponse(id(), expiresAtNanos(), reason, verb());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's test this function as well. Is verb() returning request verb or the verb of the new response message we have built?

Copy link
Copy Markdown
Collaborator

@niksajakovljevic niksajakovljevic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add test for failureResponse(RequestFailureReason reason) to make sure we use the correct verb().

@MSpryszynski
Copy link
Copy Markdown
Author

LGTM. Can you please inspect the test failures (for some reason I can't access Butler atm).

4 of them have open ticket with the same failure stack. Other 2 are failing for other open PRs too.

@cassci-bot
Copy link
Copy Markdown

❌ Build ds-cassandra-pr-gate/PR-2244 rejected by Butler


7 regressions found
See build details here


Found 7 new test failures

Test Explanation Runs Upstream
o.a.c.index.sai.cql.VectorCompaction100dTest.testCompactionWithEnoughRowsForPQAndDeleteARow[ec false] REGRESSION 🔴 0 / 18
o.a.c.index.sai.cql.VectorSiftSmallTest.testMultiSegmentBuild[ec true false] REGRESSION 🔴 0 / 18
o.a.c.index.sai.cql.VectorUpdateDeleteTest.testTTLOverwriteHasCorrectOnDiskRowCount[fa true true] (compression) REGRESSION 🔴🔵 0 / 18
o.a.c.net.ConnectionTest.testTimeout REGRESSION 🔴🔴 4 / 18
o.a.c.net.MessageTest.testNonStaticFailureResponse (compression) NEW 🔵🔴 0 / 18
o.a.c.net.ProxyHandlerConnectionsTest.suddenDisconnect (compression) REGRESSION 🔴🔴 4 / 18
o.a.c.net.ProxyHandlerConnectionsTest.testExpireSomeFromBatch (compression) REGRESSION 🔴🔴 4 / 18

Found 2 known test failures

@sonarqubecloud
Copy link
Copy Markdown

@niksajakovljevic niksajakovljevic self-requested a review April 9, 2026 12:28
@MSpryszynski MSpryszynski merged commit fc994ad into main Apr 9, 2026
2 of 4 checks passed
@MSpryszynski MSpryszynski deleted the cndb-16583 branch April 9, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants