From 1462dc7af828924b2cee556789bfa6eff6e017c0 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Fri, 20 Oct 2023 14:38:26 -0400 Subject: [PATCH 1/7] Make TableUpdateValidator composable in a sequence of operations. --- .../table/impl/TableUpdateValidator.java | 27 ++++++++++--- .../engine/table/impl/ErrorListener.java | 1 - .../engine/table/impl/QueryTableTest.java | 6 +++ .../deephaven/engine/testutil/EvalNugget.java | 19 ++++++--- .../testutil/UpdateValidatorNugget.java | 39 ++++++++++++------- 5 files changed, 65 insertions(+), 27 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java index 1dc5cbfb770..67d90cb9800 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java @@ -19,6 +19,7 @@ import io.deephaven.util.SafeCloseableList; import org.apache.commons.lang3.mutable.MutableInt; +import javax.sound.midi.Track; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -48,8 +49,9 @@ public static TableUpdateValidator make(final String description, final QueryTab private final ModifiedColumnSet validationMCS; private ColumnInfo[] columnInfos; - private WritableRowSet rowSet; + private TrackingWritableRowSet rowSet; private QueryTable resultTable; + private ModifiedColumnSet.Transformer transformer; private SharedContext sharedContext; private final String description; @@ -97,11 +99,10 @@ public String getLogPrefix() { @Override public Result initialize(boolean usePrev, long beforeClock) { - rowSet = usePrev ? tableToValidate.getRowSet().copyPrev() : tableToValidate.getRowSet().copy(); + rowSet = (usePrev ? tableToValidate.getRowSet().copyPrev() : tableToValidate.getRowSet().copy()).toTracking(); - resultTable = new QueryTable(RowSetFactory.empty().toTracking(), - Collections.emptyMap()); - resultTable.setFlat(); + resultTable = new QueryTable(rowSet, tableToValidate.getColumnSourceMap()); + transformer = tableToValidate.newModifiedColumnSetIdentityTransformer(resultTable); final TableUpdateListener listener; try (final SafeCloseable ignored1 = maybeOpenSharedContext(); @@ -146,7 +147,7 @@ public void deepValidation() { } private void onUpdate(final TableUpdate upstream) { - if (resultTable.size() >= MAX_ISSUES) { + if (issues.size() >= MAX_ISSUES) { return; } @@ -212,7 +213,13 @@ private void onUpdate(final TableUpdate upstream) { } result.append("\n"); resultTable.notifyListenersOnError(new RuntimeException(result.toString()), null); + return; } + + final TableUpdateImpl downstream = TableUpdateImpl.copy(upstream); + downstream.modifiedColumnSet = resultTable.getModifiedColumnSetForUpdates(); + transformer.transform(upstream.modifiedColumnSet(), downstream.modifiedColumnSet); + resultTable.notifyListeners(downstream); } } @@ -242,6 +249,14 @@ private void noteIssue(Supplier issue) { } } + /** + * Has an update validation failed on this table? + * @return true if an update validation has failed on this table. + */ + public boolean hasFailed() { + return !issues.isEmpty(); + } + private void validateValues(final String what, final ModifiedColumnSet columnsToCheck, final RowSet toValidate, final boolean usePrev, final boolean invertMCS) { try (final RowSequence.Iterator it = toValidate.getRowSequenceIterator()) { diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java index e96f0ddfaf0..245436eb194 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java @@ -17,7 +17,6 @@ public ErrorListener(Table table) { @Override public void onUpdate(final TableUpdate upstream) { - TestCase.fail("Should not have gotten an update!"); } @Override diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java index e08172271ca..9c8ce50d1a1 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java @@ -1464,6 +1464,9 @@ public void testSnapshotDependencies() { // Now we should flush the second snapshot flushed = updateGraph.flushOneNotificationForUnitTests(); TestCase.assertTrue(flushed); + // Which also generates a result notification as a pass-through + flushed = updateGraph.flushOneNotificationForUnitTests(); + TestCase.assertTrue(flushed); TestCase.assertTrue( snappedFirst.satisfied(ExecutionContext.getContext().getUpdateGraph().clock().currentStep())); TestCase.assertTrue( @@ -1474,6 +1477,9 @@ public void testSnapshotDependencies() { // This should flush the second TUV flushed = updateGraph.flushOneNotificationForUnitTests(); TestCase.assertTrue(flushed); + // Which also generates a result notification as a pass-through + flushed = updateGraph.flushOneNotificationForUnitTests(); + TestCase.assertTrue(flushed); // And now we should be done flushed = updateGraph.flushOneNotificationForUnitTests(); diff --git a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java index cdfc4ee5993..e5333e0b9bf 100644 --- a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java +++ b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java @@ -52,10 +52,7 @@ public EvalNugget(String description) { private Throwable exception = null; // We should listen for failures on the table, and if we get any, the test case is no good. - class FailureListener extends InstrumentedTableUpdateListener { - FailureListener() { - super("Failure Listener"); - } + class FailureListener extends ValidationFailureListener { @Override public void onUpdate(final TableUpdate upstream) { @@ -64,6 +61,17 @@ public void onUpdate(final TableUpdate upstream) { System.out.println(upstream); } } + } + + class ValidationFailureListener extends InstrumentedTableUpdateListener { + ValidationFailureListener() { + super("Failure Listener"); + } + + @Override + public void onUpdate(TableUpdate upstream) { + // do nothing + } @Override public void onFailureInternal(Throwable originalException, Entry sourceEntry) { @@ -88,10 +96,11 @@ public void onFailureInternal(Throwable originalException, Entry sourceEntry) { } private final TableUpdateValidator validator; + private final TableUpdateListener validationFailureListener = new ValidationFailureListener(); { if (originalValue instanceof QueryTable && ((QueryTable) originalValue).isRefreshing()) { validator = TableUpdateValidator.make((QueryTable) originalValue); - validator.getResultTable().addUpdateListener(failureListener); + validator.getResultTable().addUpdateListener(validationFailureListener); } else { validator = null; } diff --git a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/UpdateValidatorNugget.java b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/UpdateValidatorNugget.java index d042c457aa4..8857f6b2e17 100644 --- a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/UpdateValidatorNugget.java +++ b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/UpdateValidatorNugget.java @@ -10,6 +10,7 @@ import io.deephaven.engine.table.impl.QueryTable; import io.deephaven.engine.table.impl.TableUpdateValidator; import io.deephaven.engine.util.TableTools; +import io.deephaven.util.annotations.ReferentialIntegrity; import junit.framework.TestCase; import org.junit.Assert; @@ -27,7 +28,7 @@ public UpdateValidatorNugget(final QueryTable table) { this.validator = TableUpdateValidator.make(originalValue); originalValue.addUpdateListener(failureListener); - validator.getResultTable().addUpdateListener(failureListener); + validator.getResultTable().addUpdateListener(validatorFailureListener); } private final QueryTable originalValue; @@ -36,26 +37,34 @@ public UpdateValidatorNugget(final QueryTable table) { private Throwable exception = null; // We should listen for failures on the table, and if we get any, the test case is no good. - private final TableUpdateListener failureListener = - new InstrumentedTableUpdateListener("Failure Listener") { - @Override - public void onUpdate(TableUpdate update) {} - - @Override - public void onFailureInternal(Throwable originalException, Entry sourceEntry) { - exception = originalException; - final StringWriter errors = new StringWriter(); - originalException.printStackTrace(new PrintWriter(errors)); - TestCase.fail(errors.toString()); - } - }; + @ReferentialIntegrity + private final TableUpdateListener failureListener = new FailureListener(); + @ReferentialIntegrity + private final TableUpdateListener validatorFailureListener = new FailureListener(); public void validate(final String msg) { Assert.assertNull(exception); - Assert.assertEquals(0, validator.getResultTable().size()); + Assert.assertFalse(validator.hasFailed()); } public void show() { TableTools.showWithRowSet(originalValue, 100); } + + private class FailureListener extends InstrumentedTableUpdateListener { + public FailureListener() { + super("Failure Listener"); + } + + @Override + public void onUpdate(TableUpdate update) {} + + @Override + public void onFailureInternal(Throwable originalException, Entry sourceEntry) { + exception = originalException; + final StringWriter errors = new StringWriter(); + originalException.printStackTrace(new PrintWriter(errors)); + TestCase.fail(errors.toString()); + } + } } From c7b471c476b33603a156be10c947fa0d17da57ad Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Fri, 20 Oct 2023 14:51:14 -0400 Subject: [PATCH 2/7] Spotless. --- .../io/deephaven/engine/table/impl/TableUpdateValidator.java | 1 + .../java/io/deephaven/engine/table/impl/ErrorListener.java | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java index 67d90cb9800..68aac62d6ec 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java @@ -251,6 +251,7 @@ private void noteIssue(Supplier issue) { /** * Has an update validation failed on this table? + * * @return true if an update validation has failed on this table. */ public boolean hasFailed() { diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java index 245436eb194..afdefae8f64 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java @@ -16,8 +16,7 @@ public ErrorListener(Table table) { } @Override - public void onUpdate(final TableUpdate upstream) { - } + public void onUpdate(final TableUpdate upstream) {} @Override public void onFailureInternal(Throwable originalException, Entry sourceEntry) { From 5a7601d05f4b8fff84100c9058f40b101569a5f4 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Fri, 20 Oct 2023 16:45:26 -0400 Subject: [PATCH 3/7] optimize imports --- .../table/impl/TableUpdateValidator.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java index 68aac62d6ec..65663a2886a 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java @@ -4,25 +4,28 @@ package io.deephaven.engine.table.impl; import io.deephaven.base.verify.Assert; +import io.deephaven.chunk.Chunk; +import io.deephaven.chunk.WritableBooleanChunk; +import io.deephaven.chunk.WritableObjectChunk; import io.deephaven.chunk.attributes.Values; +import io.deephaven.chunk.util.hashing.ChunkEquals; import io.deephaven.configuration.Configuration; import io.deephaven.datastructures.util.CollectionUtil; -import io.deephaven.engine.rowset.*; -import io.deephaven.engine.rowset.RowSetFactory; +import io.deephaven.engine.rowset.RowSequence; +import io.deephaven.engine.rowset.RowSet; +import io.deephaven.engine.rowset.RowSetShiftData; +import io.deephaven.engine.rowset.TrackingWritableRowSet; import io.deephaven.engine.table.*; -import io.deephaven.vector.*; -import io.deephaven.chunk.util.hashing.ChunkEquals; import io.deephaven.engine.table.impl.sources.SparseArrayColumnSource; -import io.deephaven.chunk.*; -import io.deephaven.engine.table.impl.util.*; +import io.deephaven.engine.table.impl.util.ChunkUtils; +import io.deephaven.engine.table.impl.util.ShiftData; import io.deephaven.util.SafeCloseable; import io.deephaven.util.SafeCloseableList; +import io.deephaven.vector.*; import org.apache.commons.lang3.mutable.MutableInt; -import javax.sound.midi.Track; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.function.Supplier; public class TableUpdateValidator implements QueryTable.Operation { From 80769efac4a3611ac5625a03d8351e0a1ae285f3 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Sat, 21 Oct 2023 02:20:09 -0400 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Ryan Caudy --- .../io/deephaven/engine/table/impl/TableUpdateValidator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java index 65663a2886a..8eba6caacfb 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java @@ -101,8 +101,8 @@ public String getLogPrefix() { } @Override - public Result initialize(boolean usePrev, long beforeClock) { - rowSet = (usePrev ? tableToValidate.getRowSet().copyPrev() : tableToValidate.getRowSet().copy()).toTracking(); + public Result initialize(boolean usePrev, long beforeClock) { + rowSet = (usePrev ? tableToValidate.getRowSet().prev() : tableToValidate.getRowSet()).copy().toTracking(); resultTable = new QueryTable(rowSet, tableToValidate.getColumnSourceMap()); transformer = tableToValidate.newModifiedColumnSetIdentityTransformer(resultTable); From b4cf8641f9fc309c7b3185f379491285312b0c09 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Sat, 21 Oct 2023 02:22:20 -0400 Subject: [PATCH 5/7] code review --- .../java/io/deephaven/engine/table/impl/ErrorListener.java | 1 - .../main/java/io/deephaven/engine/testutil/EvalNugget.java | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java index afdefae8f64..1b93fe8bdf8 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/ErrorListener.java @@ -5,7 +5,6 @@ import io.deephaven.engine.table.Table; import io.deephaven.engine.table.TableUpdate; -import junit.framework.TestCase; public class ErrorListener extends InstrumentedTableUpdateListenerAdapter { diff --git a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java index e5333e0b9bf..74be7b2be2f 100644 --- a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java +++ b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/EvalNugget.java @@ -96,13 +96,15 @@ public void onFailureInternal(Throwable originalException, Entry sourceEntry) { } private final TableUpdateValidator validator; - private final TableUpdateListener validationFailureListener = new ValidationFailureListener(); + private final TableUpdateListener validationFailureListener; { if (originalValue instanceof QueryTable && ((QueryTable) originalValue).isRefreshing()) { validator = TableUpdateValidator.make((QueryTable) originalValue); + validationFailureListener = new ValidationFailureListener(); validator.getResultTable().addUpdateListener(validationFailureListener); } else { validator = null; + validationFailureListener = null; } } From c541ba1566e25be523b60fa0ad95556b526edcb3 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Sat, 21 Oct 2023 02:33:39 -0400 Subject: [PATCH 6/7] ditch transformer --- .../io/deephaven/engine/table/impl/TableUpdateValidator.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java index 8eba6caacfb..3b1a9dfbfe7 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java @@ -54,7 +54,6 @@ public static TableUpdateValidator make(final String description, final QueryTab private TrackingWritableRowSet rowSet; private QueryTable resultTable; - private ModifiedColumnSet.Transformer transformer; private SharedContext sharedContext; private final String description; @@ -105,7 +104,6 @@ public Result initialize(boolean usePrev, long beforeClock) { rowSet = (usePrev ? tableToValidate.getRowSet().prev() : tableToValidate.getRowSet()).copy().toTracking(); resultTable = new QueryTable(rowSet, tableToValidate.getColumnSourceMap()); - transformer = tableToValidate.newModifiedColumnSetIdentityTransformer(resultTable); final TableUpdateListener listener; try (final SafeCloseable ignored1 = maybeOpenSharedContext(); @@ -220,8 +218,6 @@ private void onUpdate(final TableUpdate upstream) { } final TableUpdateImpl downstream = TableUpdateImpl.copy(upstream); - downstream.modifiedColumnSet = resultTable.getModifiedColumnSetForUpdates(); - transformer.transform(upstream.modifiedColumnSet(), downstream.modifiedColumnSet); resultTable.notifyListeners(downstream); } } From e00b0b6dfd5cc966f65b5f92641dcf33c1ef7fee Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Sat, 21 Oct 2023 02:36:50 -0400 Subject: [PATCH 7/7] generic --- .../io/deephaven/engine/table/impl/TableUpdateValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java index 3b1a9dfbfe7..5f2190f2b29 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java @@ -28,7 +28,7 @@ import java.util.Arrays; import java.util.function.Supplier; -public class TableUpdateValidator implements QueryTable.Operation { +public class TableUpdateValidator implements QueryTable.Operation { private static final boolean useSharedContext = Configuration.getInstance() .getBooleanForClassWithDefault(TableUpdateValidator.class, "useSharedContext", true); private static final boolean aggressiveUpdateValidation = Configuration.getInstance()