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.

Sign up for GitHub

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

Jump to bottom

db: iterator creation is fallible #2994

Closed
jbowens opened this issue Oct 11, 2023 · 0 comments · Fixed by #3004
Closed

db: iterator creation is fallible #2994

jbowens opened this issue Oct 11, 2023 · 0 comments · Fixed by #3004
Assignees
@jbowens
Labels
A-storage C-bug Something isn't working T-storage

Comments

@jbowens
Copy link
Collaborator

jbowens commented Oct 11, 2023

Relates to #1115.

The pebble Iterator is generally designed to tolerate errors. If an iterator operation errors, resulting in Iterator.Error() returning a non-nil error, the expectation is that the user may seek the Iterator again, and the previous error is cleared. There's at least 1 violation of this contract in which an I/O error encountered during iterator construction results in an Iterator that will always error.

If L0 contains any range keys, each file in L0 containing the range keys is opened during iterator construction. If opening the file encounters an error, then a keyspan.Iter that always errors is used, preventing any Iterator operation from succeeding:

pebble/range_keys.go

Line 73 in 36b3f57

i.rangeKey.iterConfig.AddLevel(&errorKeyspanIter{err: err})

We should not perform any I/O during the iterator creation itself.
@jbowens jbowens added the C-bug Something isn't working label Oct 11, 2023
jbowens added a commit to jbowens/pebble that referenced this issue Oct 11, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
e868e07 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 11, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
8599e21 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
@jbowens jbowens mentioned this issue Oct 11, 2023
Merged
jbowens added a commit to jbowens/pebble that referenced this issue Oct 11, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
afd0b22 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 11, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
affd954 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 12, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
a0cf4d1 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 12, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
317b29e 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 13, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
cf90bbb 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 13, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
0d02bb7 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 13, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
c9fdb2b 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 13, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
afb3916 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small DSL. Future work will build off this to test handling of I/O
errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 16, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
cbc150f 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small lisp-like DSL. Future work will build off this to test handling
of I/O errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 16, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
01544e3 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small lisp-like DSL. Future work will build off this to test handling
of I/O errors during iteration.

Informs cockroachdb#1115.
Informs cockroachdb#2994.
@jbowens jbowens self-assigned this Oct 16, 2023
jbowens added a commit that referenced this issue Oct 16, 2023
@jbowens
vfs/errorfs: add facilities for error injection in datadriven tests
94ccf35 
We frequently use the errorfs package to inject errors into filesystem
operations to test error code paths. This commit builds off this package to
support encoding error injection conditions within datadriven tests through
parsing a small lisp-like DSL. Future work will build off this to test handling
of I/O errors during iteration.

Informs #1115.
Informs #2994.
jbowens added a commit to jbowens/pebble that referenced this issue Oct 17, 2023
@jbowens
db: defer loading L0 range key blocks during iterator construction
07287c9 
Previously, construction of an iterator over range keys that found sstables
containing range keys within L0 performed I/O to load the range key blocks
during iterator construction. This was less efficient: If the iterator
ultimately didn't need to read the keyspace overlapping the sstables containing
range keys, the block loads were unnecessary.

More significantly, if the I/O failed during iterator construction, the
resulting iterator was unusable. It would always error with the original error
returned by the failed block load. This is a deviation from iterator error
handling across the rest of the iterator stack, which allows an Iterator to be
re-seeked to clear the current iterator error.

Resolves cockroachdb#2994.
@jbowens jbowens mentioned this issue Oct 17, 2023
Merged
jbowens added a commit to jbowens/pebble that referenced this issue Oct 17, 2023
@jbowens
db: defer loading L0 range key blocks during iterator construction
31c81a4 
Previously, construction of an iterator over range keys that found sstables
containing range keys within L0 performed I/O to load the range key blocks
during iterator construction. This was less efficient: If the iterator
ultimately didn't need to read the keyspace overlapping the sstables containing
range keys, the block loads were unnecessary.

More significantly, if the I/O failed during iterator construction, the
resulting iterator was unusable. It would always error with the original error
returned by the failed block load. This is a deviation from iterator error
handling across the rest of the iterator stack, which allows an Iterator to be
re-seeked to clear the current iterator error.

Resolves cockroachdb#2994.
jbowens added a commit that referenced this issue Oct 20, 2023
@jbowens
db: defer loading L0 range key blocks during iterator construction
babd592 
Previously, construction of an iterator over range keys that found sstables
containing range keys within L0 performed I/O to load the range key blocks
during iterator construction. This was less efficient: If the iterator
ultimately didn't need to read the keyspace overlapping the sstables containing
range keys, the block loads were unnecessary.

More significantly, if the I/O failed during iterator construction, the
resulting iterator was unusable. It would always error with the original error
returned by the failed block load. This is a deviation from iterator error
handling across the rest of the iterator stack, which allows an Iterator to be
re-seeked to clear the current iterator error.

Resolves #2994.
@jbowens jbowens moved this to Done in [Deprecated] Storage Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jbowens jbowens

Labels
A-storage C-bug Something isn't working T-storage
Projects
[Deprecated] Storage
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

db: defer loading L0 range key blocks during iterator construction jbowens/pebble
1 participant
@jbowens