Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CASSANDRA-19477 trunk dont go to disk for hints sizes #3180

Closed
wants to merge 1 commit into from

Conversation

smiklosovic
Copy link
Contributor

Thanks for sending a pull request! Here are some tips if you're new here:

  • Ensure you have added or run the appropriate tests for your PR.
  • Be sure to keep the PR description updated to reflect all changes.
  • Write your PR title to summarize what this PR proposes.
  • If possible, provide a concise example to reproduce the issue for a faster review.
  • Read our contributor guidelines
  • If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

@smiklosovic smiklosovic force-pushed the CASSANDRA-19477-trunk branch 11 times, most recently from 5e55b19 to c1db700 Compare March 18, 2024 07:20
@clohfink
Copy link
Contributor

+1 thanks for this!

@iamaleksey
Copy link
Member

I would just cache the size value in a transient field in HintsDescriptor. A CHM is an overkill here.

@smiklosovic smiklosovic force-pushed the CASSANDRA-19477-trunk branch 5 times, most recently from 415531a to 816129f Compare March 20, 2024 11:39
@@ -271,6 +273,7 @@ public void close() throws IOException
flushBuffer();
maybeFsync();
maybeSkipCache();
descriptor.size = position();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

why not to just update it in close() ? Then when this is used in try-with-resources in HintsWriteExecutor, we do not need to change there anything at all and not mess with finally etc ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be probably in finally though, not sure ... as all above can throw

@smiklosovic smiklosovic marked this pull request as ready for review March 20, 2024 22:33
@smiklosovic smiklosovic force-pushed the CASSANDRA-19477-trunk branch 6 times, most recently from 26a70d3 to 84129a4 Compare March 22, 2024 17:46
patch by Stefan Miklosovic; reviewed by Aleksey Yeschenko, Chris Lohfink for CASSANDRA-19477
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants