JAkutenshi commented on code in PR #5391:
URL: https://github.com/apache/ignite-3/pull/5391#discussion_r1989977205
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/TableManagerTest.java:
##########
@@ -429,6 +431,8 @@ public void
testWriteTableAssignmentsToMetastoreExceptionally() throws Exception
* @throws Exception If failed.
*/
@Test
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-24397 - remove
@WithSystemProperty.
Review Comment:
Should we propagate this TODO for all annotations below? Compromise is to
add `remove @WithSystemProperty there and below.`
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/storage/InternalTableEstimatedSizeTest.java:
##########
@@ -144,6 +146,10 @@ public class InternalTableEstimatedSizeTest extends
BaseIgniteAbstractTest {
private MessagingService messagingService;
+ private static ReplicationGroupId replicationGroupId(int partId) {
Review Comment:
TODO on colocation cleanup ticket?
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/TableManagerTest.java:
##########
@@ -429,6 +431,8 @@ public void
testWriteTableAssignmentsToMetastoreExceptionally() throws Exception
* @throws Exception If failed.
*/
@Test
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-24397 - remove
@WithSystemProperty.
+ @WithSystemProperty(key = COLOCATION_FEATURE_FLAG, value = "false")
Review Comment:
Am I correct that all this unit tests are disabled for colocation branch and
we should provide colocation alternative for them?
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/PartitionCommandListenerTest.java:
##########
@@ -393,6 +395,7 @@ public void testInsertRowsBatchedAndCheck() {
}
@Test
+ @WithSystemProperty(key = COLOCATION_FEATURE_FLAG, value = "false")
Review Comment:
TODO?
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -2975,21 +2988,32 @@ private void cleanup(UUID txId) {
}
private void cleanup(UUID txId, boolean commit) {
- HybridTimestamp commitTs = clock.now();
+ TxState newTxState = commit ? COMMITTED : ABORTED;
- txManager.updateTxMeta(txId, old -> new TxStateMeta(COMMITTED,
UUID.randomUUID(), commitPartitionId, commitTs, null));
+ HybridTimestamp commitTs = clock.now();
+ HybridTimestamp commitTsOrNull = commit ? commitTs : null;
- WriteIntentSwitchReplicaRequest message =
TX_MESSAGES_FACTORY.writeIntentSwitchReplicaRequest()
- .groupId(tablePartitionIdMessage(grpId))
- .tableIds(Set.of(grpId.tableId()))
- .txId(txId)
- .commit(commit)
- .commitTimestamp(commitTs)
- .build();
+ txManager.updateTxMeta(txId, old -> new TxStateMeta(newTxState,
UUID.randomUUID(), commitPartitionId, commitTsOrNull, null));
- assertThat(partitionReplicaListener.invoke(message, localNode.id()),
willCompleteSuccessfully());
+ if (enabledColocation()) {
+ lockManager.releaseAll(txId);
+ partitionReplicaListener.cleanupLocally(txId, commit, commitTs);
Review Comment:
Assert works only in non-colocation branch, otherwise nothing will check? Or
I miss something?
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -742,6 +744,7 @@ private void reset() {
}
@Test
+ @WithSystemProperty(key = COLOCATION_FEATURE_FLAG, value = "false")
Review Comment:
TODO?
--
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
