Do not hide errors, which trigger manual delta check point file read#28808
Merged
ebyhr merged 1 commit intotrinodb:masterfrom Mar 25, 2026
Merged
Do not hide errors, which trigger manual delta check point file read#28808ebyhr merged 1 commit intotrinodb:masterfrom
ebyhr merged 1 commit intotrinodb:masterfrom
Conversation
raphaelsolarski
approved these changes
Mar 23, 2026
ebyhr
reviewed
Mar 23, 2026
| // it could be situation, like access deny or other permission failure error, which actually should not trigger manual check point read | ||
| // because we can not be sure, which exceptions could appear here, at least we should not silently hide them | ||
| // TODO after we collect more of such situations we could add exclusion rules here | ||
| log.warn(e, "Failed to read Delta Lake last checkpoint file %s, falling back to manual checkpoint discovery", checkpointPath); |
Member
There was a problem hiding this comment.
Why warning log? What is the expected action when users see this message?
Contributor
Author
There was a problem hiding this comment.
hi @ebyhr, because of this comment :
// but some file system implementations
// will throw different exceptions if the checkpoint is not found
we can not be 100% sure that this will bring us to the point, where reading check point file manually will fail.
But generally should.
Imagine situation (before this PR), some changes on directory access permission
- we got exception here - strict permission error
- silently hide it (because we think that it's legal and we just need to find latest check point manually) and return Optional.empty() here
- we try to find it and read
- and got completely another exception smth like
Metadata not found in transaction log for, because we hide original permission error.
So with this change user will be able to understand what is original cause.
Maybe log level should be error, but in this case some legit exceptional situation will be highlighted as error.
Member
|
Could you fix error-prone failure? |
It could be situation, like access deny or other permission failure error, which actually should not trigger manual check point read, because we can not be sure, which exceptions could appear here - at least we should not silently hide them, after we collect more of such situations we could add exclusion rules here
vlad-lyutenko
force-pushed
the
vlad-lyutenko/fix-delta-log
branch
from
4edf22a to
3a530ff
Compare
ebyhr
approved these changes
Mar 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
It could be situation, like access deny or other permission failure error, which actually should not trigger manual check point read, because we can not be sure, which exceptions could appear here - at least we should not silently hide them, after we collect more of such situations we could add exclusion rules here
Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.