vaidikgarg-bit commented on code in PR #3445:
URL: https://github.com/apache/hugegraph/pull/3445#discussion_r4436367036
##########
hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/business/itrv2/MultiListIterator.java:
##########
@@ -115,21 +127,53 @@ public boolean isValid() {
*/
@Override
public void close() {
- if (this.innerIterator != null) {
- this.innerIterator.close();
+ if (this.closed) {
+ return;
}
+ this.closed = true;
+ ScanIterator current = this.innerIterator;
+ this.innerIterator = null;
if (this.innerListIterator == null) {
// never iterated, so every sub-iterator is still open
this.innerListIterator = this.iterators.iterator();
}
+
+ // keep closing the rest if one fails, then rethrow the first failure
+ Throwable failure = null;
+ if (current != null) {
+ failure = closeSource(current, failure);
+ }
while (this.innerListIterator.hasNext()) {
- this.innerListIterator.next().close();
+ failure = closeSource(this.innerListIterator.next(), failure);
}
this.iterators.clear();
+ if (failure instanceof RuntimeException runtimeFailure) {
+ throw runtimeFailure;
+ }
+ if (failure != null) {
+ throw (Error) failure;
+ }
+ }
+
+ private static Throwable closeSource(ScanIterator iterator, Throwable
failure) {
+ try {
+ iterator.close();
+ } catch (RuntimeException | Error e) {
+ if (failure == null) {
+ return e;
+ }
+ if (failure != e) {
+ failure.addSuppressed(e);
+ }
+ }
+ return failure;
}
@Override
public <T> T next() {
+ if (!hasNext()) {
Review Comment:
Thanks, done. `hasNext()` now records that it found an element, and `next()`
and repeated `hasNext()`/`isValid()` calls use that record instead of probing
the child again. `testMultiListProbesAdvancingSourceOncePerRow` uses a child
whose `hasNext()` advances and replaces the pending row. It fails on the
previous head, and on master too, since master's `hasNext()` probes the child
again on every call. It passes now. This also removes the extra per-row probe I
noted in the description.
On impact: the extra probe in `next()` came from my previous commit. Since
#3424 the two `InAccurate*` iterators keep their pending element (`if (current
!= null) return true;`), and the other children I checked in
`BusinessHandlerImpl` are also safe to probe twice, so I don't think it dropped
rows with the current children. It did add a probe per row and relied on that
idempotence, and now it doesn't.
--
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]
