vaidikgarg-bit opened a new pull request, #3445:
URL: https://github.com/apache/hugegraph/pull/3445
## Purpose of the PR
- close #3444
#3437 made `MultiListIterator.close()` close every sub-iterator when it runs
before the first `hasNext()`. Three problems remained in the same class:
1. `close()` closes the sub-iterators with no `try`/`finally`. When one
`close()` throws, the remaining sub-iterators stay open and the list is never
cleared.
2. `next()` dereferences `innerIterator`, which is only set by
`hasNext()`/`isValid()`. Calling `next()` first, or after `hasNext()` has
returned `false`, throws `NullPointerException`. Repeated `next()` calls
without `hasNext()` also never move past the current sub-iterator.
3. `close()` leaves `innerIterator` pointing at the source it just closed.
The next `hasNext()` or `isValid()` calls into that closed source and can close
it again.
## Main Changes
- Add a `closed` flag, as `UnionFilterIterator` and
`IntersectionFilterIterator` have. Once it is set, `hasNext()` and `isValid()`
return `false` without touching any source, and a second `close()` does nothing.
- `close()` drops its reference to the current sub-iterator and keeps going
when a sub-iterator's `close()` throws a `RuntimeException` or an `Error`
(`RocksDBScanIterator.close()` rethrows both). It clears the list and then
rethrows the first failure, with any later failures added as suppressed
exceptions.
- `next()` calls `hasNext()` first, so it moves on to the next sub-iterator
by itself and throws `NoSuchElementException` when nothing is left, including
after `close()`. With the usual `hasNext()`/`next()` loop this costs one extra
`hasNext()` on the current sub-iterator per row (for `RocksDBScanIterator`, one
more `isOwningHandle()`/`isValid()`/`key()` round trip and key filter).
Not changed here: `position()` has no closed check, so after closing
mid-iteration it now throws `NullPointerException` instead of reading the
closed source (before iteration it already threw on master). A close failure
raised while `hasNext()` moves past an exhausted sub-iterator is also left as
is.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests
here)*.
- [x] Need tests and can be verified as follows:
- five new tests in `FilterIteratorContractTest`: a `RuntimeException`
close failure before iteration, an `Error` close failure before iteration, a
close failure during iteration, `next()` without `hasNext()`, and
`hasNext()`/`isValid()`/`next()`/`close()` after closing mid-iteration. All
five fail on master.
- `store-server-test` (202), `store-core-test` (113) and
`store-rocksdb-test` (19) pass locally on `8e327b8f3`, and `mvn
editorconfig:check` is clean for the changed modules.
## Does this PR potentially affect the following parts?
- [ ] Dependencies ([add/update
license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses)
info &
[regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh))
- [ ] Modify configurations
- [ ] The public API
- [ ] Other affects (typed here)
- [x] Nope
## Documentation Status
Select one option and provide the documentation location when applicable.
- [ ] `Doc - TODO`: required documentation is pending; complete it before
merging.
- [ ] `Doc - Done`: documentation is included here or linked below.
- [x] `Doc - No Need`: no user-visible documentation is affected.
Documentation files in this PR or paired hugegraph-doc PR:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
