maksaska commented on code in PR #12913:
URL: https://github.com/apache/ignite/pull/12913#discussion_r3259085933
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotSelfTest.java:
##########
@@ -903,6 +904,18 @@ private void checkIncrementalSnapshotWalRecords(IgniteEx
node, IncrementalSnapsh
}
}
+ /** Print thread dump if {@code IgniteFutureTimeoutException} is raised. */
+ protected void runWithLogggedThreadDump(Runnable action) {
Review Comment:
Loggged
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteClusterSnapshotHandlerTest.java:
##########
@@ -61,6 +61,9 @@ public class IgniteClusterSnapshotHandlerTest extends
IgniteClusterSnapshotResto
/** Custom snapshot handlers. */
private final List<SnapshotHandler<?>> handlers = new ArrayList<>();
+ /** Timeout in milliseconds to await for snapshot operation being
completed. */
+ protected static final long TIMEOUT = 60_000;
Review Comment:
Could you clarify the scope of this PR? The ticket description mentions
covering only Disk 4 tests, but this PR modifies
IgniteClusterSnapshotHandlerTest, which belongs to the Disk 5 suite.
- If we are including Disk 5: Please update the ticket description to avoid
confusion. Also, can we update the timeouts for IgniteSnapshotMXBeanTest while
we are at it?
- If Disk 5 is out of scope: I suggest reverting the changes to
IgniteClusterSnapshotHandlerTest to keep this PR focused.
##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteClusterSnapshotCheckWithIndexesTest.java:
##########
@@ -36,6 +36,9 @@
* Cluster-wide snapshot test check command with indexes.
*/
public class IgniteClusterSnapshotCheckWithIndexesTest extends
AbstractSnapshotSelfTest {
+ /** Timeout in milliseconds to await for snapshot operation being
completed. */
+ protected static final long TIMEOUT = 60_000;
Review Comment:
Same question as for IgniteClusterSnapshotHandlerTest.
The ticket description mentions covering only Disk 4 tests, but this PR
modifies IgniteClusterSnapshotCheckWithIndexingTest as well as
IgniteClusterSnapshotRestoreWithIndexingTest, which belongs to the Disk 6 suite.
- If we are including Disk 6: Please update the ticket description to avoid
confusion. Also, can we update the timeouts for
IgniteClusterSnapshotMetricsTest while we are at it? From what I've seen on
teamcity, the timeout trick didn't work for
IgniteClusterSnapshotRestoreWithIndexingTest
- If Disk 6 is out of scope: I suggest reverting the changes to keep this PR
focused.
##########
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteClusterSnapshotRestoreWithIndexingTest.java:
##########
@@ -74,7 +74,8 @@ public void testBasicClusterSnapshotRestore() throws
Exception {
IgniteEx client = startGridsWithSnapshot(2, CACHE_KEYS_RANGE, true);
- grid(0).snapshot().restoreSnapshot(SNAPSHOT_NAME,
Collections.singleton(DEFAULT_CACHE_NAME)).get(TIMEOUT);
+ runWithLogggedThreadDump(() ->
+ grid(0).snapshot().restoreSnapshot(SNAPSHOT_NAME,
Collections.singleton(DEFAULT_CACHE_NAME)).get(TIMEOUT));
Review Comment:
Still relevant
--
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]
