Skip to content

Conversation

driftx
Copy link

@driftx driftx commented Aug 15, 2025

What is the issue

To ease service shutdown investigations we could log a received signal.

What does this PR fix and why was it fixed

Logs signals received.

Copy link

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

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
66.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link

@djatnieks djatnieks left a comment

Choose a reason for hiding this comment

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

It looks like we've lost the previous build results - push a commit or manually trigger the build again?

assert !isShutdown;
isShutdown = true;

logger.info("Running data daemon shutdown hook");

Choose a reason for hiding this comment

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

Should it be something like "Running StorageService shutdown hook"?

drain(false);
}

protected synchronized void drain(boolean isFinalShutdown) throws IOException, InterruptedException, ExecutionException

Choose a reason for hiding this comment

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

I noticed that if isFinalShutdown == true some messages are not logged - I wonder if it would be useful to have those logged in the context of CNDB given the ask in the ticket for more logging?

Copy link
Author

Choose a reason for hiding this comment

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

They are calling setMode though, which happens to optionally log. We could change those to not log and explicitly log instead, wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants