Check all "not found" errors #175
Merged
Conversation
This file contains 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
Currently only the leveldb not found errors are handled. If a pebble not found error is returned by the backend it is propagated, causing the batchposter to fail to send transactions. With this change all DB not found errors are handled the same way.
sveitser
requested review from
ImJeremyHe,
nomaxg,
alysiahuggins,
zacshowa,
Sneh1999 and
jbearer
as code owners
alysiahuggins
approved these changes
Jul 30, 2024
| @@ -62,7 +62,7 @@ func (s *Storage) Get(_ context.Context, index uint64) (*storage.QueuedTransacti | |||
| key := idxToKey(index) | |||
| value, err := s.db.Get(key) | |||
| if err != nil { | |||
| if errors.Is(err, leveldb.ErrNotFound) { | |||
| if isErrNotFound(err) { | |||
There was a problem hiding this comment.
good call - and are the only errors we care about catching the ones we care about in this function?
func isErrNotFound(err error) bool {
return errors.Is(err, leveldb.ErrNotFound) || errors.Is(err, pebble.ErrNotFound) || errors.Is(err, memorydb.ErrMemorydbNotFound)
}
There was a problem hiding this comment.
I don't know for sure but I believe the implementation of the function that was changed in this PR is supposed to hide the "not found" errors for all db backend implementations, and only these errors.
|
Upstream PR: |
Sneh1999
approved these changes
Jul 30, 2024
|
Not sure why CI is failing. Upstream merged the PR already. |
zacshowa
pushed a commit
that referenced
this pull request
Nov 26, 2024
Add unit test for conditional OSP
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently only the leveldb not found errors are handled. If a pebble not found error is returned by the backend it is propagated, causing the batchposter to fail to send transactions.
With this change all DB not found errors are handled the same way.