Skip to content

Commit 577b4d4

Browse files
authored
Don't close long-lived RowSets in Downsampler BucketState (deephaven#5478)
Fixes a bug in downsampling where a table may cause an error if freed while processing an update. Fixes deephaven#5476
1 parent 5e0964e commit 577b4d4

File tree

2 files changed

+7
-16
lines changed

2 files changed

+7
-16
lines changed

ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/BucketState.java

+7-9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
import io.deephaven.engine.rowset.RowSetFactory;
1010
import io.deephaven.engine.rowset.impl.RowSetUtils;
1111
import io.deephaven.engine.rowset.chunkattributes.OrderedRowKeys;
12+
import io.deephaven.internal.log.LoggerFactory;
13+
import io.deephaven.io.logger.Logger;
1214
import io.deephaven.util.QueryConstants;
1315
import io.deephaven.chunk.Chunk;
1416
import io.deephaven.chunk.LongChunk;
@@ -26,6 +28,8 @@
2628
* its own offset in those arrays.
2729
*/
2830
public class BucketState {
31+
private static final Logger log = LoggerFactory.getLogger(BucketState.class);
32+
2933
private final WritableRowSet rowSet = RowSetFactory.empty();
3034

3135
private RowSet cachedRowSet;
@@ -310,22 +314,16 @@ public void validate(final boolean usePrev, final DownsampleChunkContext context
310314
values[columnIndex].validate(offset, keyChunk.get(indexInChunk), valueChunks[columnIndex],
311315
indexInChunk, trackNulls ? nulls[columnIndex] : null);
312316
} catch (final RuntimeException e) {
313-
System.out.println(rowSet);
314317
final String msg =
315318
"Bad data! indexInChunk=" + indexInChunk + ", col=" + columnIndex + ", usePrev="
316-
+ usePrev + ", offset=" + offset + ", rowSet=" + keyChunk.get(indexInChunk);
319+
+ usePrev + ", offset=" + offset + ", indexInChunk="
320+
+ keyChunk.get(indexInChunk);
321+
log.error().append(msg).append(", rowSet=").append(rowSet).endl();
317322
throw new IllegalStateException(msg, e);
318323
}
319324
}
320325
}
321326
}
322327
Assert.eqTrue(makeRowSet().subsetOf(rowSet), "makeRowSet().subsetOf(rowSet)");
323328
}
324-
325-
public void close() {
326-
if (cachedRowSet != null) {
327-
cachedRowSet.close();
328-
}
329-
rowSet.close();
330-
}
331329
}

ClientSupport/src/main/java/io/deephaven/clientsupport/plotdownsampling/RunChartDownsample.java

-7
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,6 @@ private DownsamplerListener(
317317
allYColumnIndexes = IntStream.range(0, key.yColumnNames.length).toArray();
318318
}
319319

320-
@Override
321-
protected void destroy() {
322-
super.destroy();
323-
states.values().forEach(BucketState::close);
324-
}
325-
326320
@Override
327321
public void onUpdate(final TableUpdate upstream) {
328322
try (final DownsampleChunkContext context =
@@ -684,7 +678,6 @@ private void performRescans(final DownsampleChunkContext context) {
684678
// if it has no keys at all, remove it so we quit checking it
685679
iterator.remove();
686680
releasePosition(bucket.getOffset());
687-
bucket.close();
688681
} else {
689682
bucket.rescanIfNeeded(context);
690683
}

0 commit comments

Comments
 (0)