You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/12/20 15:59:45 UTC
[GitHub] [ignite-3] tkalkirill opened a new pull request, #1462: IGNITE-18024 Implementation of a full rebalance for TxStateRocksDbStorage on receiver
tkalkirill opened a new pull request, #1462:
URL: https://github.com/apache/ignite-3/pull/1462
https://issues.apache.org/jira/browse/IGNITE-18024
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill merged pull request #1462: IGNITE-18022 Creating an API for full rebalance of TxStateStorage on receiver
Posted by GitBox <gi...@apache.org>.
tkalkirill merged PR #1462:
URL: https://github.com/apache/ignite-3/pull/1462
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1462: IGNITE-18022 Creating an API for full rebalance of TxStateStorage on receiver
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #1462:
URL: https://github.com/apache/ignite-3/pull/1462#discussion_r1055239240
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxStateStorage.java:
##########
@@ -122,9 +125,55 @@ public interface TxStateStorage extends ManuallyCloseable {
void close();
/**
- * Removes all data from the storage.
+ * Closes and removes all data from the storage.
*
- * @throws StorageException In case when the operation has failed.
+ * @throws IgniteInternalException with {@link Transactions#TX_STATE_STORAGE_ERR} error code in case when the operation has failed.
*/
void destroy();
+
+ /**
+ * Prepares the transaction state storage for a full rebalance: clears the storage, sets the {@link #lastAppliedIndex()} and
+ * {@link #lastAppliedTerm()} to {@link #FULL_REBALANCE_IN_PROGRESS}, and closes all cursors.
+ *
+ * <p>After calling this method, only write methods will be available, and read methods with {@link #lastApplied(long, long)} will
+ * throw {@link IgniteInternalException}.
Review Comment:
Fix it.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1462: IGNITE-18022 Creating an API for full rebalance of TxStateStorage on receiver
Posted by GitBox <gi...@apache.org>.
ibessonov commented on code in PR #1462:
URL: https://github.com/apache/ignite-3/pull/1462#discussion_r1055486814
##########
modules/transactions/src/testFixtures/java/org/apache/ignite/internal/tx/storage/state/test/TestTxStateStorage.java:
##########
@@ -100,11 +100,31 @@ public void remove(UUID txId) {
public Cursor<IgniteBiTuple<UUID, TxMeta>> scan() {
checkFullRebalanceInProgress();
- List<IgniteBiTuple<UUID, TxMeta>> copy = storage.entrySet().stream()
+ Iterator<IgniteBiTuple<UUID, TxMeta>> iterator = storage.entrySet().stream()
.map(e -> new IgniteBiTuple<>(e.getKey(), e.getValue()))
- .collect(toList());
+ .collect(toList())
+ .iterator();
- return Cursor.fromIterable(copy);
+ return new Cursor<>() {
+ @Override
+ public void close() {
+ // No-op.
+ }
+
+ @Override
+ public boolean hasNext() {
+ assert fullRebalanceFutureReference.get() == null;
Review Comment:
Why do we even need it?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1462: IGNITE-18022 Creating an API for full rebalance of TxStateStorage on receiver
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #1462:
URL: https://github.com/apache/ignite-3/pull/1462#discussion_r1055192159
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxStateStorage.java:
##########
@@ -122,9 +125,55 @@ public interface TxStateStorage extends ManuallyCloseable {
void close();
/**
- * Removes all data from the storage.
+ * Closes and removes all data from the storage.
*
- * @throws StorageException In case when the operation has failed.
+ * @throws IgniteInternalException with {@link Transactions#TX_STATE_STORAGE_ERR} error code in case when the operation has failed.
*/
void destroy();
+
+ /**
+ * Prepares the transaction state storage for a full rebalance: clears the storage, sets the {@link #lastAppliedIndex()} and
+ * {@link #lastAppliedTerm()} to {@link #FULL_REBALANCE_IN_PROGRESS}, and closes all cursors.
+ *
+ * <p>After calling this method, only write methods will be available, and read methods with {@link #lastApplied(long, long)} will
+ * throw {@link IgniteInternalException}.
+ *
+ * <p>This method must be called before every full rebalance of transaction state storage and ends with a call to one of the methods:
+ * <ul>
+ * <li>{@link #abortFullRebalance()} - in case of errors or cancellation of a full rebalance;</li>
+ * <li>{@link #finishFullRebalance(long, long)} - in case of successful completion of a full rebalance.</li>
+ * </ul>
+ *
+ * <p>If the {@link #lastAppliedIndex()} is {@link #FULL_REBALANCE_IN_PROGRESS} after a node restart, then the storage needs to be
+ * cleared before it can be used.
Review Comment:
You're right, we can make it automatic at the start of the storage, I think it's worth mentioning.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1462: IGNITE-18022 Creating an API for full rebalance of TxStateStorage on receiver
Posted by GitBox <gi...@apache.org>.
ibessonov commented on code in PR #1462:
URL: https://github.com/apache/ignite-3/pull/1462#discussion_r1054439523
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxStateStorage.java:
##########
@@ -122,9 +125,55 @@ public interface TxStateStorage extends ManuallyCloseable {
void close();
/**
- * Removes all data from the storage.
+ * Closes and removes all data from the storage.
*
- * @throws StorageException In case when the operation has failed.
+ * @throws IgniteInternalException with {@link Transactions#TX_STATE_STORAGE_ERR} error code in case when the operation has failed.
*/
void destroy();
+
+ /**
+ * Prepares the transaction state storage for a full rebalance: clears the storage, sets the {@link #lastAppliedIndex()} and
+ * {@link #lastAppliedTerm()} to {@link #FULL_REBALANCE_IN_PROGRESS}, and closes all cursors.
+ *
+ * <p>After calling this method, only write methods will be available, and read methods with {@link #lastApplied(long, long)} will
+ * throw {@link IgniteInternalException}.
Review Comment:
Can we have a more specific exception? Maybe a link to the corresponding error code? Let's make our exceptions better.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxStateStorage.java:
##########
@@ -122,9 +125,55 @@ public interface TxStateStorage extends ManuallyCloseable {
void close();
/**
- * Removes all data from the storage.
+ * Closes and removes all data from the storage.
*
- * @throws StorageException In case when the operation has failed.
+ * @throws IgniteInternalException with {@link Transactions#TX_STATE_STORAGE_ERR} error code in case when the operation has failed.
*/
void destroy();
+
+ /**
+ * Prepares the transaction state storage for a full rebalance: clears the storage, sets the {@link #lastAppliedIndex()} and
+ * {@link #lastAppliedTerm()} to {@link #FULL_REBALANCE_IN_PROGRESS}, and closes all cursors.
+ *
+ * <p>After calling this method, only write methods will be available, and read methods with {@link #lastApplied(long, long)} will
+ * throw {@link IgniteInternalException}.
+ *
+ * <p>This method must be called before every full rebalance of transaction state storage and ends with a call to one of the methods:
+ * <ul>
+ * <li>{@link #abortFullRebalance()} - in case of errors or cancellation of a full rebalance;</li>
+ * <li>{@link #finishFullRebalance(long, long)} - in case of successful completion of a full rebalance.</li>
+ * </ul>
+ *
+ * <p>If the {@link #lastAppliedIndex()} is {@link #FULL_REBALANCE_IN_PROGRESS} after a node restart, then the storage needs to be
+ * cleared before it can be used.
Review Comment:
Ok, how do you clean it using this interface? Maybe we shouldn't mention it like this, because cleaning happens automatically during the storage start, isn't it?
##########
modules/transactions/src/testFixtures/java/org/apache/ignite/internal/tx/storage/state/AbstractTxStateStorageTest.java:
##########
@@ -257,53 +247,178 @@ public void scanOnlySeesDataExistingAtTheMomentOfCreation() throws Exception {
List<UUID> txIdsReturnedByScan = cursor.stream()
.map(IgniteBiTuple::getKey)
.collect(toList());
+
assertThat(txIdsReturnedByScan, is(List.of(existingBeforeScan)));
}
-
- partitionStorage.close();
Review Comment:
This implies that partitions are closed by the table, right? That's cool
##########
modules/transactions/src/testFixtures/java/org/apache/ignite/internal/tx/storage/state/AbstractTxStateStorageTest.java:
##########
@@ -257,53 +247,178 @@ public void scanOnlySeesDataExistingAtTheMomentOfCreation() throws Exception {
List<UUID> txIdsReturnedByScan = cursor.stream()
.map(IgniteBiTuple::getKey)
.collect(toList());
+
assertThat(txIdsReturnedByScan, is(List.of(existingBeforeScan)));
}
-
- partitionStorage.close();
}
@Test
void lastAppliedIndexGetterIsConsistentWithSetter() {
TxStateStorage partitionStorage = tableStorage.getOrCreateTxStateStorage(0);
- try {
- partitionStorage.lastApplied(10, 2);
+ partitionStorage.lastApplied(10, 2);
- assertThat(partitionStorage.lastAppliedIndex(), is(10L));
- } finally {
- partitionStorage.close();
- }
+ assertThat(partitionStorage.lastAppliedIndex(), is(10L));
}
@Test
void lastAppliedTermGetterIsConsistentWithSetter() {
TxStateStorage partitionStorage = tableStorage.getOrCreateTxStateStorage(0);
- try {
- partitionStorage.lastApplied(10, 2);
+ partitionStorage.lastApplied(10, 2);
- assertThat(partitionStorage.lastAppliedTerm(), is(2L));
- } finally {
- partitionStorage.close();
- }
+ assertThat(partitionStorage.lastAppliedTerm(), is(2L));
}
@Test
void compareAndSetMakesLastAppliedChangeVisible() {
TxStateStorage partitionStorage = tableStorage.getOrCreateTxStateStorage(0);
- try {
- UUID txId = UUID.randomUUID();
- partitionStorage.compareAndSet(txId, null, randomTxMeta(1, txId), 10, 2);
+ UUID txId = UUID.randomUUID();
+ partitionStorage.compareAndSet(txId, null, randomTxMeta(1, txId), 10, 2);
+
+ assertThat(partitionStorage.lastAppliedIndex(), is(10L));
+ assertThat(partitionStorage.lastAppliedTerm(), is(2L));
+ }
+
+ @Test
+ public void testSuccessFullRebalance() throws Exception {
+ TxStateStorage storage = tableStorage.getOrCreateTxStateStorage(0);
+
+ // Nothing will happen because the full rebalance has not started.
+ storage.finishFullRebalance(100, 500).get(1, SECONDS);
Review Comment:
Are you sure that this one shouldn't throw an exception?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1462: IGNITE-18022 Creating an API for full rebalance of TxStateStorage on receiver
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #1462:
URL: https://github.com/apache/ignite-3/pull/1462#discussion_r1055192919
##########
modules/transactions/src/testFixtures/java/org/apache/ignite/internal/tx/storage/state/AbstractTxStateStorageTest.java:
##########
@@ -257,53 +247,178 @@ public void scanOnlySeesDataExistingAtTheMomentOfCreation() throws Exception {
List<UUID> txIdsReturnedByScan = cursor.stream()
.map(IgniteBiTuple::getKey)
.collect(toList());
+
assertThat(txIdsReturnedByScan, is(List.of(existingBeforeScan)));
}
-
- partitionStorage.close();
}
@Test
void lastAppliedIndexGetterIsConsistentWithSetter() {
TxStateStorage partitionStorage = tableStorage.getOrCreateTxStateStorage(0);
- try {
- partitionStorage.lastApplied(10, 2);
+ partitionStorage.lastApplied(10, 2);
- assertThat(partitionStorage.lastAppliedIndex(), is(10L));
- } finally {
- partitionStorage.close();
- }
+ assertThat(partitionStorage.lastAppliedIndex(), is(10L));
}
@Test
void lastAppliedTermGetterIsConsistentWithSetter() {
TxStateStorage partitionStorage = tableStorage.getOrCreateTxStateStorage(0);
- try {
- partitionStorage.lastApplied(10, 2);
+ partitionStorage.lastApplied(10, 2);
- assertThat(partitionStorage.lastAppliedTerm(), is(2L));
- } finally {
- partitionStorage.close();
- }
+ assertThat(partitionStorage.lastAppliedTerm(), is(2L));
}
@Test
void compareAndSetMakesLastAppliedChangeVisible() {
TxStateStorage partitionStorage = tableStorage.getOrCreateTxStateStorage(0);
- try {
- UUID txId = UUID.randomUUID();
- partitionStorage.compareAndSet(txId, null, randomTxMeta(1, txId), 10, 2);
+ UUID txId = UUID.randomUUID();
+ partitionStorage.compareAndSet(txId, null, randomTxMeta(1, txId), 10, 2);
+
+ assertThat(partitionStorage.lastAppliedIndex(), is(10L));
+ assertThat(partitionStorage.lastAppliedTerm(), is(2L));
+ }
+
+ @Test
+ public void testSuccessFullRebalance() throws Exception {
+ TxStateStorage storage = tableStorage.getOrCreateTxStateStorage(0);
+
+ // Nothing will happen because the full rebalance has not started.
+ storage.finishFullRebalance(100, 500).get(1, SECONDS);
Review Comment:
You're right, it makes sense.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1462: IGNITE-18022 Creating an API for full rebalance of TxStateStorage on receiver
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #1462:
URL: https://github.com/apache/ignite-3/pull/1462#discussion_r1055581634
##########
modules/transactions/src/testFixtures/java/org/apache/ignite/internal/tx/storage/state/test/TestTxStateStorage.java:
##########
@@ -100,11 +100,31 @@ public void remove(UUID txId) {
public Cursor<IgniteBiTuple<UUID, TxMeta>> scan() {
checkFullRebalanceInProgress();
- List<IgniteBiTuple<UUID, TxMeta>> copy = storage.entrySet().stream()
+ Iterator<IgniteBiTuple<UUID, TxMeta>> iterator = storage.entrySet().stream()
.map(e -> new IgniteBiTuple<>(e.getKey(), e.getValue()))
- .collect(toList());
+ .collect(toList())
+ .iterator();
- return Cursor.fromIterable(copy);
+ return new Cursor<>() {
+ @Override
+ public void close() {
+ // No-op.
+ }
+
+ @Override
+ public boolean hasNext() {
+ assert fullRebalanceFutureReference.get() == null;
Review Comment:
Just make sure that after the start of the rebalance, the cursor will no longer be used.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org