db: clear accumulated error in Iterator.Error #1811
Comments
nicktrav
added a commit
to nicktrav/cockroach
that referenced
this issue
Jul 25, 2022
In cockroachdb#84449, the behavior of the `pebbleIterator` was altered to ignore all errors when closing an iterator. This was done to avoid panic-ing when the iterator encounters an ephemeral (i.e. retriable) error that remains pinned to the iterator until it is closed (see cockroachdb#84396 for the motivation). Partially address the TODO added in cockroachdb#84449 by panic-ing on errors that are known to be fatal (such as corruption). Addressing the TODO in its entirety would likely require enumerating known ephemeral errors and allowing such errors to be ignored on close, as well as implementing cockroachdb/pebble#1811 to avoid pinning errors to the iterator even after they have been handled. Fix cockroachdb#84479. Release note: None.
craig bot
pushed a commit
to cockroachdb/cockroach
that referenced
this issue
Jul 25, 2022
84590: *: upgrade to go 1.18.4 r=knz a=rickystewart * [ ] Adjust the Pebble tests to run in new version. * [x] Adjust version in Docker image ([source](./builder/Dockerfile)). * [x] Adjust version in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh)) * [ ] Rebuild and push the Docker image (following [Basic Process](#basic-process)) * [x] Download ALL the archives (`.tar.gz`, `.zip`) for the new Go version from https://golang.org/dl/ and mirror them in the `public-bazel-artifacts` bucket in the `Bazel artifacts` project in GCP (sub-directory `go`, next to the other Go SDK's). * [x] Bump the version in `WORKSPACE` under `go_download_sdk`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases). Also edit the filenames listed in `sdks` and update all the hashes to match what you mirrored in the step above. * [x] Run `./dev generate bazel` to refresh `distdir_files.bzl`, then `bazel fetch `@distdir//:archives`` to ensure you've updated all hashes to the correct value. * [ ] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)). * [x] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release. * [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed. * [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)). * [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`). * [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects. * [ ] Ask the Developer Infrastructure team to deploy new TeamCity agent images according to [packer/README.md](./packer/README.md) Release note (build change): Upgrade to golang 1.18.4 84723: opt: build UDF expressions r=mgartner a=mgartner #### opt: set opt tester search path to empty `OptTester` now sets its `SemaContext`'s `SearchPath` to `EmptySearchPath`, instead of `nil`, to avoid nil pointer exceptions when resolving unknown functions. Release note: None #### opt: build UDF expressions This commit adds basic support for building UDFs in optbuilder. Only scalar, nullary (arity of zero) functions with a single statement in the body are supported. Support for more types of UDFs will follow in future commits. Note that this commit does not add support for execution of UDFs, only building them within an optimizer expression. Release note: None 84913: storage: panic on iterator close when encountering corruption r=jbowens a=nicktrav In #84449, the behavior of the `pebbleIterator` was altered to ignore all errors when closing an iterator. This was done to avoid panic-ing when the iterator encounters an ephemeral (i.e. retriable) error that remains pinned to the iterator until it is closed (see #84396 for the motivation). Partially address the TODO added in #84449 by panic-ing on errors that are known to be fatal (such as corruption). Addressing the TODO in its entirety would likely require enumerating known ephemeral errors and allowing such errors to be ignored on close, as well as implementing cockroachdb/pebble#1811 to avoid pinning errors to the iterator even after they have been handled. Fix #84479. Release note: None. 84917: outliers: rename to insights r=matthewtodd a=matthewtodd The UI design that includes outliers also features other insights about sql execution. We will expand this outliers subsystem to support making those insights as well (probably with more detectors, changing the signature / return type of `isOutlier`). Renaming now is the first step in that direction. Release note: None 84987: kvserver: send range GC requests when point requests are absent r=aliher1911 a=aliher1911 Previously range GC requests were not sent if point GC keys were not requested as well. This patch fixes the problem by checking that any of parameters could be specified. Release note: None 85011: sql: remove a reference to issue 77733 r=ajwerner a=knz Fixes #77733. Co-authored-by: Ricky Stewart <[email protected]> Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Nick Travers <[email protected]> Co-authored-by: Matthew Todd <[email protected]> Co-authored-by: Oleg Afanasyev <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
|
We have marked this issue as stale because it has been inactive for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When an iterator encounters an error during iteration, it's surfaced to the user via
Iterator.Error. If subsequently the user callsCloseto release resources, the iterator will return the same accumulated error. This makes it difficult to test for additional errors encountered during the act of closing.I think we want to preserve the behavior of returning accumulated errors in
Close, because it makes for a convenient pattern of closing the iterator when exhausted, and checking its return value to observe whether it was exhausted due to an error.If calling
Iterator.Errorcleared the accumulated error, then we could guarantee a subsequent call toIterator.Closereturns an error only if a new error was encountered.The text was updated successfully, but these errors were encountered: