sashapolo commented on code in PR #5300:
URL: https://github.com/apache/ignite-3/pull/5300#discussion_r1978901754
##########
modules/storage-api/src/main/java/org/apache/ignite/internal/storage/util/StorageUtils.java:
##########
@@ -137,6 +138,7 @@ public static void
throwExceptionDependingOnStorageStateOnRebalance(StorageState
public static void throwExceptionDependingOnStorageState(StorageState
state, String storageInfo) {
switch (state) {
case CLOSED:
+ assert !IgniteSystemProperties.enabledColocation() :
createStorageClosedErrorMessage(storageInfo);
Review Comment:
I'll explain why the current approach is weird:
1. This is an assertion that covers a line that throws an exception, which
makes no sense in the real world. The only reason for it is to make TC fail. So
the proper solution is to fix the TC behavior.
2. This method can throw 4 exceptions depending on the scenario, but we only
add an assertion only for one of them.
3. This assertion is only enabled for the colocation track and is expected
to be removed in the future, why would we suddenly stop being interested in
storages being stopped incorrectly?
So I think what you want to do is to remove this line and fix TC settings to
fail if this exception is detected in the logs.
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
