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

Tidy up journal writer code comments and error handling #8732

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fulghum
Copy link
Contributor

@fulghum fulghum commented Jan 10, 2025

Before this change, the code that iterates over journal records would silently stop processing journal records once it encountered any invalid record. This change turns those invalid records into errors so that data isn't silently ignored. Customers can opt-in to the previous behavior by setting the DOLT_SKIP_INVALID_JOURNAL_RECORDS environment variable to any value.

@fulghum fulghum marked this pull request as ready for review January 24, 2025 20:50
@dolthub dolthub deleted a comment from coffeegoddd Jan 24, 2025
Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

This seems fine, but maybe @reltuk should comment on whether there was a good reason for the old behavior. Would hate for this to start dying on an edge case we aren't aware of.

…ing out for invalid journal records, to preserve graceful crash consistency
@coffeegoddd
Copy link
Contributor

@fulghum DOLT

comparing_percentages
100.000000 to 100.000000
version result total
f06b307 ok 5937457
version total_tests
f06b307 5937457
correctness_percentage
100.0

@dolthub dolthub deleted a comment from coffeegoddd Jan 29, 2025
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