Abacn commented on code in PR #24841:
URL: https://github.com/apache/beam/pull/24841#discussion_r1059445625
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:
##########
@@ -390,13 +409,10 @@ public boolean tryClaim(TimestampedValue<T>[] position) {
}
@Override
- protected void finalize() throws Throwable {
+ public void close() throws IOException {
Review Comment:
IOException will now bubble up rather than logging, however this adds
assumption that the caller of `BoundedSourceAsSDFRestrictionTracker.close()`
will catch the Exception so that it does not fail the bundle (currently it
does).
Also note that both GC's finalize() and Beam's tearDown are best effort. The
consequence of this change of behavior in real scenario is not clear. To avoid
unexpected impact I personally conservative on core codes that currently work
property.
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:
##########
@@ -270,6 +272,8 @@ public void populateDisplayData(DisplayData.Builder
builder) {
private static final Logger LOG =
LoggerFactory.getLogger(BoundedSourceAsSDFWrapperFn.class);
private static final long DEFAULT_DESIRED_BUNDLE_SIZE_BYTES = 64 * (1 <<
20);
+ private final ArrayDeque<Closeable> restrictionTrackerStack = new
ArrayDeque<>(5);
Review Comment:
Not obvious to me why use a Dequeue of size 5 here.
--
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]
