JAkutenshi commented on code in PR #7294:
URL: https://github.com/apache/ignite-3/pull/7294#discussion_r2646162614
##########
modules/transactions/src/integrationTest/java/org/apache/ignite/internal/disaster/DisasterRecoveryTestUtil.java:
##########
@@ -125,27 +126,34 @@ static void assertValueOnSpecificNodes(
int id,
int val,
SchemaDescriptor schema
- ) throws Exception {
- for (IgniteImpl node : nodes) {
- assertValueOnSpecificNode(tableName, node, id, val, schema);
- }
+ ) {
+ CompletableFuture<?>[] futures = nodes.stream()
Review Comment:
May the follow be simpler? Assertion method is synchronous, but `runAsync`
still will use common fork-join-pool.
```
nodes.parallelStream().forEach(node -> assertValueOnSpecificNode(tableName,
node, id, val, schema));
```
##########
modules/transactions/src/integrationTest/java/org/apache/ignite/internal/disaster/DisasterRecoveryTestUtil.java:
##########
@@ -125,27 +126,34 @@ static void assertValueOnSpecificNodes(
int id,
int val,
SchemaDescriptor schema
- ) throws Exception {
- for (IgniteImpl node : nodes) {
- assertValueOnSpecificNode(tableName, node, id, val, schema);
- }
+ ) {
+ CompletableFuture<?>[] futures = nodes.stream()
+ .map(node -> CompletableFuture.runAsync(() ->
+ assertValueOnSpecificNode(tableName, node, id, val,
schema)))
+ .toArray(CompletableFuture[]::new);
+
+ CompletableFuture.allOf(futures).join();
}
- static void assertValueOnSpecificNode(String tableName, IgniteImpl node,
int id, int val, SchemaDescriptor schema) throws Exception {
+ static void assertValueOnSpecificNode(String tableName, IgniteImpl node,
int id, int val, SchemaDescriptor schema) {
InternalTable internalTable =
unwrapTableViewInternal(node.tables().table(tableName)).internalTable();
Row keyValueRow0 = createKeyValueRow(schema, id, val);
Row keyRow0 = createKeyRow(schema, id);
- assertTrue(waitForCondition(() -> {
- try {
- CompletableFuture<BinaryRow> getFut =
internalTable.get(keyRow0, node.clock().now(), node.node());
-
- return compareRows(getFut.get(), keyValueRow0);
- } catch (Exception e) {
- return false;
- }
- }, 10_000), "Row comparison failed within the timeout.");
+ try {
+ await()
+ .atMost(ofSeconds(20))
+ .until(() -> {
+ BinaryRow actual = internalTable.get(keyRow0,
node.clock().now(), node.node())
+ .join();
+
+ return compareRows(actual, keyValueRow0);
+ });
+ } catch (ConditionTimeoutException e) {
+ throw new AssertionError("Row comparison failed within the timeout
on " + node.name()
+ + " for id=" + id + ", val=" + val, e);
Review Comment:
Should we obey `[id={}, val={}].` log messages notation in tests there? In
utils class I think more as yes.
##########
modules/transactions/src/integrationTest/java/org/apache/ignite/internal/disaster/ItDisasterRecoveryReconfigurationTest.java:
##########
@@ -1522,6 +1523,8 @@ void testAssignmentsChainUpdatedOnAutomaticReset() throws
Exception {
List<Throwable> errors = insertValues(table, partId, 0);
assertThat(errors, is(empty()));
+ assertInsertedValuesOnSpecificNodes(table.name(), dataNodes, partId,
0);
Review Comment:
Suggestion to rename `dataNodes` -> `dataNodeNames`
--
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]
