You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ibilley7 (via GitHub)" <gi...@apache.org> on 2025/08/01 14:43:14 UTC
[PR] Remove Table id lock in Manager [accumulo]
ibilley7 opened a new pull request, #5773:
URL: https://github.com/apache/accumulo/pull/5773
Create test `testUniquenessOfTableId()` in TableOperationsIT to ensure that concurrent calls will get a unique table id, and that the lock in Manager can be removed.
Closes #5723
--
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@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove Table id lock in Manager [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5773:
URL: https://github.com/apache/accumulo/pull/5773#discussion_r2248841194
##########
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##########
@@ -834,4 +839,38 @@ public static void setExpectedTabletAvailability(Map<TabletId,TabletAvailability
expected.put(new TabletIdImpl(ke), availability);
}
+ @Test
+ public void testUniquenessOfTableId() throws ExecutionException, InterruptedException {
+ List<Future<TableId>> futureList = new ArrayList<>();
+
+ Set<TableId> hash = new HashSet<>();
+
+ ExecutorService pool = Executors.newFixedThreadPool(64);
+
+ for (int i = 0; i < 1000; i++) {
+ int finalI = i;
+
+ Future<TableId> future = pool.submit(() -> {
+ TableId tableId = null;
+
+ try {
+ tableId = Utils.getNextId("Testing" + finalI, getServerContext(), TableId::of);
+ } catch (Exception e) {
Review Comment:
Do not need to catch the exception here and ignore it. If an exception is thrown it will eventually cause the get() method on the future to throw an exception and that will cause the test to fail.
--
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@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove Table id lock in Manager [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on PR #5773:
URL: https://github.com/apache/accumulo/pull/5773#issuecomment-3152445810
Noticed the lock was unused now and remove it in 55b894beee30d4ae2fc2fa17b605988997982db9
--
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@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove Table id lock in Manager [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #5773:
URL: https://github.com/apache/accumulo/pull/5773
--
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@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove Table id lock in Manager [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5773:
URL: https://github.com/apache/accumulo/pull/5773#discussion_r2248381982
##########
.mvn/maven.config:
##########
@@ -1,3 +1,2 @@
-# Remove this file when the snapshot repository is no longer needed to resolve SNAPSHOT dependencies
Review Comment:
Why was this change made?
##########
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##########
@@ -834,4 +839,31 @@ public static void setExpectedTabletAvailability(Map<TabletId,TabletAvailability
expected.put(new TabletIdImpl(ke), availability);
}
+ @Test
+ public void testUniquenessOfTableId() throws ExecutionException, InterruptedException {
+ Set<TableId> hash = new HashSet<>();
+
+ ExecutorService pool = Executors.newFixedThreadPool(64);
+
+ for (int i = 0; i < 1000; i++) {
+ int finalI = i;
+
+ Future<TableId> future = pool.submit(() -> {
+ TableId tableId = null;
+
+ try {
+ tableId = Utils.getNextId("yes" + finalI, getServerContext(), TableId::of);
+ } catch (Exception e) {
+ System.out.println(e.getMessage());
+ }
+ return tableId;
+ });
+
+ hash.add(future.get());
Review Comment:
This will wait for each future to complete before starting another, preventing concurrency. Need two loops, in the first loop run through and create all the futures and store them in a list. In the second loop, run through the list of futures and call get() on all the futures and add the to the hash set. That will allow them to run concurrently.
--
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@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove Table id lock in Manager [accumulo]
Posted by "ibilley7 (via GitHub)" <gi...@apache.org>.
ibilley7 commented on code in PR #5773:
URL: https://github.com/apache/accumulo/pull/5773#discussion_r2248536268
##########
.mvn/maven.config:
##########
@@ -1,3 +1,2 @@
-# Remove this file when the snapshot repository is no longer needed to resolve SNAPSHOT dependencies
Review Comment:
I forgot to change that back. There were issues with Maven and before I updated Maven to the most recent version, deleting this comment was what allowed mvn to run.
--
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@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org