You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "ibessonov (via GitHub)" <gi...@apache.org> on 2023/05/31 10:18:10 UTC
[GitHub] [ignite-3] ibessonov opened a new pull request, #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
ibessonov opened a new pull request, #2124:
URL: https://github.com/apache/ignite-3/pull/2124
(no comment)
--
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 #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#discussion_r1212710567
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1231,11 +1252,7 @@ private void dropTableLocally(long causalityToken, String name, int tblId, List<
removeStorageFromGcFutures[p] = mvGc.removeStorage(replicationGroupId);
}
- tablesByIdVv.update(causalityToken, (previousVal, e) -> inBusyLock(busyLock, () -> {
- if (e != null) {
- return failedFuture(e);
- }
-
+ Function<Map<Integer, TableImpl>, CompletableFuture<Map<Integer, TableImpl>>> dropTableClosure = previousVal -> {
Review Comment:
I reverted this change
--
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 pull request #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#issuecomment-1571461236
It's been decided to have additional assignments VV instead of having two VVs with the same map. This way we'll hopefully make the code more clear
--
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 #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#discussion_r1211848629
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -794,6 +792,7 @@ private CompletableFuture<?> updateAssignmentInternal(ConfigurationNotificationE
startGroupFut
.thenComposeAsync(v -> inBusyLock(busyLock, () -> {
try {
+ //TODO This procedure takes 10 seconds if there's no majority online.
Review Comment:
Thanks! I will create the issue tomorrow
--
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] denis-chudov commented on a diff in pull request #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#discussion_r1214256957
##########
modules/core/src/test/java/org/apache/ignite/internal/causality/CompletableVersionedValueTest.java:
##########
@@ -267,6 +267,18 @@ public void testDefaultValue() throws Exception {
assertEquals(TEST_VALUE, fut2.get());
}
+ @Test
+ public void testLatest() {
+ CompletableVersionedValue<Integer> vv = new CompletableVersionedValue<>();
+
+ assertEquals(-1, vv.latestCausalityToken());
+
+ vv.complete(1, 10);
+
+ assertEquals(10, vv.latest());
+ assertEquals(1, vv.latestCausalityToken());
Review Comment:
```suggestion
assertEquals(1, vv.latestCausalityToken());
vv.complete(2);
assertEquals(10, vv.latest());
assertEquals(2, vv.latestCausalityToken());
```
--
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 #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#discussion_r1214264628
##########
modules/core/src/test/java/org/apache/ignite/internal/causality/CompletableVersionedValueTest.java:
##########
@@ -267,6 +267,18 @@ public void testDefaultValue() throws Exception {
assertEquals(TEST_VALUE, fut2.get());
}
+ @Test
+ public void testLatest() {
+ CompletableVersionedValue<Integer> vv = new CompletableVersionedValue<>();
+
+ assertEquals(-1, vv.latestCausalityToken());
+
+ vv.complete(1, 10);
+
+ assertEquals(10, vv.latest());
+ assertEquals(1, vv.latestCausalityToken());
Review Comment:
Thank you!
--
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 #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#discussion_r1211743847
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -794,6 +792,7 @@ private CompletableFuture<?> updateAssignmentInternal(ConfigurationNotificationE
startGroupFut
.thenComposeAsync(v -> inBusyLock(busyLock, () -> {
try {
+ //TODO This procedure takes 10 seconds if there's no majority online.
Review Comment:
Empty 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ibessonov merged pull request #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov merged PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124
--
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 #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#discussion_r1212710045
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1112,11 +1124,7 @@ private CompletableFuture<?> createTableLocally(long causalityToken, String name
// TODO: IGNITE-19082 Need another way to wait for indexes
table.addIndexesToWait(collectTableIndexIds(tblId));
- tablesByIdVv.update(causalityToken, (previous, e) -> inBusyLock(busyLock, () -> {
- if (e != null) {
- return failedFuture(e);
- }
-
+ Function<Map<Integer, TableImpl>, CompletableFuture<Map<Integer, TableImpl>>> createTableClosure = previous -> {
Review Comment:
I reverted this change
--
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] sashapolo commented on a diff in pull request #2124: IGNITE-19593 Optimized update assignments routine to start multiple tables at the same time.
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #2124:
URL: https://github.com/apache/ignite-3/pull/2124#discussion_r1211758839
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1112,11 +1124,7 @@ private CompletableFuture<?> createTableLocally(long causalityToken, String name
// TODO: IGNITE-19082 Need another way to wait for indexes
table.addIndexesToWait(collectTableIndexIds(tblId));
- tablesByIdVv.update(causalityToken, (previous, e) -> inBusyLock(busyLock, () -> {
- if (e != null) {
- return failedFuture(e);
- }
-
+ Function<Map<Integer, TableImpl>, CompletableFuture<Map<Integer, TableImpl>>> createTableClosure = previous -> {
Review Comment:
Please rename the parameter
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1231,11 +1252,7 @@ private void dropTableLocally(long causalityToken, String name, int tblId, List<
removeStorageFromGcFutures[p] = mvGc.removeStorage(replicationGroupId);
}
- tablesByIdVv.update(causalityToken, (previousVal, e) -> inBusyLock(busyLock, () -> {
- if (e != null) {
- return failedFuture(e);
- }
-
+ Function<Map<Integer, TableImpl>, CompletableFuture<Map<Integer, TableImpl>>> dropTableClosure = previousVal -> {
Review Comment:
Same here
--
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