Skip to content

Conversation

@maedhroz
Copy link
Contributor

No description provided.

Copy link
Contributor

@yifan-c yifan-c left a comment

Choose a reason for hiding this comment

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

I think the PR does not cover all callers that can throw BufferOverflowException. Do we only care about the 2 places and convert it to PartitionSerializationException? For the rest, it just throw as BufferOverflowException.

@maedhroz
Copy link
Contributor Author

the PR does not cover all callers that can throw BufferOverflowException

@yifan-c Let me do a quick audit of the other things that might have to catch BOE.

…t its maximum size, and this allows us to catch it specifically during partition serialization both during compaction and read response creation, where we can throw a more helpful error.
- removed dead checkThrowsRuntimeException() method
- switched to getString()
- added a couple quick tests around the error message format
@maedhroz
Copy link
Contributor Author

@yifan-c I've addressed your review comments and pushed one more commit. (Tests are in progress...)

I didn't expand the use of PartitionSerializationException, as serializing partitions to disk seems to be handled well enough by catching in BigTableWriter and serializing partitions over the wire in UnfilteredRowIteratorSerializer.

adelapena pushed a commit to adelapena/cassandra that referenced this pull request Oct 13, 2023
ekaterinadimitrova2 pushed a commit to ekaterinadimitrova2/cassandra that referenced this pull request Jun 3, 2024
michaelsembwever pushed a commit to thelastpickle/cassandra that referenced this pull request Jan 7, 2026
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