kbendick commented on code in PR #4863:
URL: https://github.com/apache/iceberg/pull/4863#discussion_r882979586
##########
core/src/main/java/org/apache/iceberg/util/Tasks.java:
##########
@@ -88,8 +89,17 @@ public UnrecoverableException(Throwable cause) {
private long maxSleepTimeMs = 600000; // 10 minutes
private long maxDurationMs = 600000; // 10 minutes
private double scaleFactor = 2.0; // exponential
+ private boolean hasStarted = false;
+ /**
+ * @deprecated Use {@link #Builder(Iterator)} instead.
Review Comment:
Nit: normally we add `@deprecated since <version>[, will be removed in
<version>]; Use ....`.
I think you can use `@deprecated since 0.14.0`. As to when / if we remove
this, I wouldn't worry about it too much (as I'm not sure).
##########
core/src/main/java/org/apache/iceberg/util/Tasks.java:
##########
@@ -205,11 +215,12 @@ private <E extends Exception> boolean runSingleThreaded(
List<I> succeeded = Lists.newArrayList();
List<Throwable> exceptions = Lists.newArrayList();
- Iterator<I> iterator = items.iterator();
+ Preconditions.checkState(!hasStarted, "Cannot run twice");
+ hasStarted = true;
Review Comment:
Nit: normally we only use `this.` for field when mutating them. In most
cases, this winds up just being the constructor or when set in the builder.
I'm not sure if we would use it here or not.
I wouldn't block on this by any means, but figured I'd mention it 🙂
##########
core/src/main/java/org/apache/iceberg/util/Tasks.java:
##########
@@ -88,8 +89,17 @@ public UnrecoverableException(Throwable cause) {
private long maxSleepTimeMs = 600000; // 10 minutes
private long maxDurationMs = 600000; // 10 minutes
private double scaleFactor = 2.0; // exponential
+ private boolean hasStarted = false;
Review Comment:
Since this can be updated in `runParallel`, should we consider making this
an `AtomicBoolean` like the booleans defined inside of `runParallel` are?
--
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]
