From 7640e5101c6b80d2186907ed26e9678bda2f8cf6 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 10 May 2024 13:33:29 -0500 Subject: [PATCH 1/2] Don't close long-lived RowSets Fixes a bug in downsampling where a table may cause an error if freed while processing an update. Fixes #5476 --- .../plotdownsampling/BucketState.java | 15 ++++++--------- .../plotdownsampling/RunChartDownsample.java | 7 ------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java b/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java index f29276dca14..0c9faf49a21 100644 --- a/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java +++ b/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java @@ -9,6 +9,8 @@ import io.deephaven.engine.rowset.RowSetFactory; import io.deephaven.engine.rowset.impl.RowSetUtils; import io.deephaven.engine.rowset.chunkattributes.OrderedRowKeys; +import io.deephaven.internal.log.LoggerFactory; +import io.deephaven.io.logger.Logger; import io.deephaven.util.QueryConstants; import io.deephaven.chunk.Chunk; import io.deephaven.chunk.LongChunk; @@ -26,6 +28,8 @@ * its own offset in those arrays. */ public class BucketState { + private static final Logger log = LoggerFactory.getLogger(BucketState.class); + private final WritableRowSet rowSet = RowSetFactory.empty(); private RowSet cachedRowSet; @@ -310,10 +314,10 @@ public void validate(final boolean usePrev, final DownsampleChunkContext context values[columnIndex].validate(offset, keyChunk.get(indexInChunk), valueChunks[columnIndex], indexInChunk, trackNulls ? nulls[columnIndex] : null); } catch (final RuntimeException e) { - System.out.println(rowSet); final String msg = "Bad data! indexInChunk=" + indexInChunk + ", col=" + columnIndex + ", usePrev=" - + usePrev + ", offset=" + offset + ", rowSet=" + keyChunk.get(indexInChunk); + + usePrev + ", offset=" + offset + ", indexInChunk=" + keyChunk.get(indexInChunk); + log.error().append(msg).append("rowSet=").append(rowSet).endl(); throw new IllegalStateException(msg, e); } } @@ -321,11 +325,4 @@ public void validate(final boolean usePrev, final DownsampleChunkContext context } Assert.eqTrue(makeRowSet().subsetOf(rowSet), "makeRowSet().subsetOf(rowSet)"); } - - public void close() { - if (cachedRowSet != null) { - cachedRowSet.close(); - } - rowSet.close(); - } } diff --git a/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/RunChartDownsample.java b/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/RunChartDownsample.java index c0fadd7f2c4..3636333d8e7 100644 --- a/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/RunChartDownsample.java +++ b/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/RunChartDownsample.java @@ -317,12 +317,6 @@ private DownsamplerListener( allYColumnIndexes = IntStream.range(0, key.yColumnNames.length).toArray(); } - @Override - protected void destroy() { - super.destroy(); - states.values().forEach(BucketState::close); - } - @Override public void onUpdate(final TableUpdate upstream) { try (final DownsampleChunkContext context = @@ -684,7 +678,6 @@ private void performRescans(final DownsampleChunkContext context) { // if it has no keys at all, remove it so we quit checking it iterator.remove(); releasePosition(bucket.getOffset()); - bucket.close(); } else { bucket.rescanIfNeeded(context); } From a6a31b89f6c5ac9aea2da0b678e2c1de6adb24af Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 10 May 2024 13:51:58 -0500 Subject: [PATCH 2/2] Spotless and log cleanup --- .../clientsupport/plotdownsampling/BucketState.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java b/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java index 0c9faf49a21..420e859215b 100644 --- a/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java +++ b/ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java @@ -316,8 +316,9 @@ public void validate(final boolean usePrev, final DownsampleChunkContext context } catch (final RuntimeException e) { final String msg = "Bad data! indexInChunk=" + indexInChunk + ", col=" + columnIndex + ", usePrev=" - + usePrev + ", offset=" + offset + ", indexInChunk=" + keyChunk.get(indexInChunk); - log.error().append(msg).append("rowSet=").append(rowSet).endl(); + + usePrev + ", offset=" + offset + ", indexInChunk=" + + keyChunk.get(indexInChunk); + log.error().append(msg).append(", rowSet=").append(rowSet).endl(); throw new IllegalStateException(msg, e); } }