adoroszlai commented on code in PR #10227:
URL: https://github.com/apache/ozone/pull/10227#discussion_r3254117788
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java:
##########
@@ -27,13 +28,98 @@
*/
public class ManagedReadOptions extends ReadOptions {
private final UncheckedAutoCloseable leakTracker = track(this);
+ private final byte[] lowerBound;
+ private final byte[] upperBound;
+ private final ManagedSlice lowerBoundSlice;
+ private final ManagedSlice upperBoundSlice;
+
+ public ManagedReadOptions() {
+ this(null, null);
+ }
+
+ public ManagedReadOptions(byte[] lowerBound, byte[] upperBound) {
+ this.lowerBound = copy(lowerBound);
+ this.upperBound = copy(upperBound);
+
+ ManagedSlice lowerSlice = null;
+ ManagedSlice upperSlice = null;
+ try {
+ lowerSlice = this.lowerBound != null
+ ? new ManagedSlice(this.lowerBound) : null;
+ upperSlice = this.upperBound != null
+ ? new ManagedSlice(this.upperBound) : null;
+
+ if (lowerSlice != null) {
+ setIterateLowerBound(lowerSlice);
+ }
+ if (upperSlice != null) {
+ setIterateUpperBound(upperSlice);
+ }
+ } catch (RuntimeException | Error e) {
+ closeOnConstructorFailure(lowerSlice, upperSlice, e);
+ throw e;
+ }
+
+ this.lowerBoundSlice = lowerSlice;
+ this.upperBoundSlice = upperSlice;
+ }
+
+ private static byte[] copy(byte[] bytes) {
+ return bytes != null ? Arrays.copyOf(bytes, bytes.length) : null;
+ }
+
+ public byte[] getLowerBound() {
+ return copy(lowerBound);
+ }
+
+ public byte[] getUpperBound() {
+ return copy(upperBound);
+ }
+
+ private void closeOnConstructorFailure(ManagedSlice lowerSlice,
+ ManagedSlice upperSlice, Throwable failure) {
+ try {
+ super.close();
+ } catch (RuntimeException | Error e) {
+ failure.addSuppressed(e);
+ }
+ closeAndSuppress(lowerSlice, failure);
+ closeAndSuppress(upperSlice, failure);
+ try {
+ leakTracker.close();
+ } catch (RuntimeException | Error e) {
+ failure.addSuppressed(e);
+ }
+ }
+
+ private static void closeAndSuppress(ManagedSlice slice, Throwable failure) {
+ if (slice != null) {
+ try {
+ slice.close();
+ } catch (RuntimeException | Error e) {
+ failure.addSuppressed(e);
Review Comment:
We can make it more generic, and reuse for closing `leakTracker`.
```suggestion
private static void closeAndSuppress(AutoCloseable closeable, Throwable
failure) {
if (closeable != null) {
try {
closeable.close();
} catch (Throwable t) {
failure.addSuppressed(t);
```
If you agree, please move to `IOUtils`.
##########
hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java:
##########
@@ -27,13 +28,98 @@
*/
public class ManagedReadOptions extends ReadOptions {
private final UncheckedAutoCloseable leakTracker = track(this);
+ private final byte[] lowerBound;
+ private final byte[] upperBound;
+ private final ManagedSlice lowerBoundSlice;
+ private final ManagedSlice upperBoundSlice;
+
+ public ManagedReadOptions() {
+ this(null, null);
+ }
+
+ public ManagedReadOptions(byte[] lowerBound, byte[] upperBound) {
+ this.lowerBound = copy(lowerBound);
+ this.upperBound = copy(upperBound);
+
+ ManagedSlice lowerSlice = null;
+ ManagedSlice upperSlice = null;
+ try {
+ lowerSlice = this.lowerBound != null
+ ? new ManagedSlice(this.lowerBound) : null;
+ upperSlice = this.upperBound != null
+ ? new ManagedSlice(this.upperBound) : null;
+
+ if (lowerSlice != null) {
+ setIterateLowerBound(lowerSlice);
+ }
+ if (upperSlice != null) {
+ setIterateUpperBound(upperSlice);
+ }
+ } catch (RuntimeException | Error e) {
+ closeOnConstructorFailure(lowerSlice, upperSlice, e);
+ throw e;
+ }
+
+ this.lowerBoundSlice = lowerSlice;
+ this.upperBoundSlice = upperSlice;
+ }
+
+ private static byte[] copy(byte[] bytes) {
+ return bytes != null ? Arrays.copyOf(bytes, bytes.length) : null;
+ }
+
+ public byte[] getLowerBound() {
+ return copy(lowerBound);
+ }
+
+ public byte[] getUpperBound() {
+ return copy(upperBound);
+ }
+
+ private void closeOnConstructorFailure(ManagedSlice lowerSlice,
+ ManagedSlice upperSlice, Throwable failure) {
+ try {
+ super.close();
+ } catch (RuntimeException | Error e) {
+ failure.addSuppressed(e);
+ }
+ closeAndSuppress(lowerSlice, failure);
+ closeAndSuppress(upperSlice, failure);
+ try {
+ leakTracker.close();
+ } catch (RuntimeException | Error e) {
+ failure.addSuppressed(e);
+ }
+ }
+
+ private static void closeAndSuppress(ManagedSlice slice, Throwable failure) {
+ if (slice != null) {
+ try {
+ slice.close();
+ } catch (RuntimeException | Error e) {
+ failure.addSuppressed(e);
+ }
+ }
+ }
@Override
public void close() {
try {
super.close();
} finally {
- leakTracker.close();
+ try {
+ if (lowerBoundSlice != null) {
+ lowerBoundSlice.close();
+ }
+ } finally {
+ try {
+ if (upperBoundSlice != null) {
+ upperBoundSlice.close();
+ }
+ } finally {
+ leakTracker.close();
+ }
+ }
Review Comment:
Can be simplified?
```java
IOUtils.closeQuietly(lowerBoundSlice, upperBoundSlice, leakTracker);
```
(needs import)
--
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]
