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 2021/08/02 14:59:47 UTC

[GitHub] [ignite-3] ptupitsyn opened a new pull request #253: IGNITE-15218 Add async methods to IgniteTables

ptupitsyn opened a new pull request #253:
URL: https://github.com/apache/ignite-3/pull/253


   


-- 
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] ptupitsyn commented on a change in pull request #253: IGNITE-15218 Add async methods to IgniteTables

Posted by GitBox <gi...@apache.org>.
ptupitsyn commented on a change in pull request #253:
URL: https://github.com/apache/ignite-3/pull/253#discussion_r681149784



##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
##########
@@ -650,21 +668,39 @@ else if (tblFut.complete(tbl))
             }
         }
 
-        dropTblFut.join();
+        return dropTblFut;
     }
 
     /** {@inheritDoc} */
     @Override public List<Table> tables() {
-        ArrayList<Table> tables = new ArrayList<>();
+        return tablesAsync().join();
+    }
 
-        for (String tblName : tableNamesConfigured()) {
-            Table tbl = table(tblName, false);
+    /** {@inheritDoc} */
+    @Override public CompletableFuture<List<Table>> tablesAsync() {
+        var tableNames = tableNamesConfigured();
+        var tableFuts = new CompletableFuture[tableNames.size()];
+        var i = 0;
 
-            if (tbl != null)
-                tables.add(tbl);
-        }
+        for (String tblName : tableNames)
+            tableFuts[i++] = tableAsync(tblName, false);
+
+        return CompletableFuture.allOf(tableFuts).thenApply(unused -> {
+            var tables = new ArrayList<Table>(tableNames.size());
+
+            try {
+                for (var fut : tableFuts) {
+                    var table = fut.get();

Review comment:
       Streams are nice but come at a cost of performance and extra allocations. Even though `tables()` API is unlikely to be used in a tight loop, I'd prefer to keep the simple loop here with proper `ArrayList` capacity constructor call.




-- 
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] AMashenkov commented on a change in pull request #253: IGNITE-15218 Add async methods to IgniteTables

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #253:
URL: https://github.com/apache/ignite-3/pull/253#discussion_r681076490



##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
##########
@@ -650,21 +668,39 @@ else if (tblFut.complete(tbl))
             }
         }
 
-        dropTblFut.join();
+        return dropTblFut;
     }
 
     /** {@inheritDoc} */
     @Override public List<Table> tables() {
-        ArrayList<Table> tables = new ArrayList<>();
+        return tablesAsync().join();
+    }
 
-        for (String tblName : tableNamesConfigured()) {
-            Table tbl = table(tblName, false);
+    /** {@inheritDoc} */
+    @Override public CompletableFuture<List<Table>> tablesAsync() {
+        var tableNames = tableNamesConfigured();
+        var tableFuts = new CompletableFuture[tableNames.size()];
+        var i = 0;
 
-            if (tbl != null)
-                tables.add(tbl);
-        }
+        for (String tblName : tableNames)
+            tableFuts[i++] = tableAsync(tblName, false);
+
+        return CompletableFuture.allOf(tableFuts).thenApply(unused -> {
+            var tables = new ArrayList<Table>(tableNames.size());
+
+            try {
+                for (var fut : tableFuts) {
+                    var table = fut.get();

Review comment:
       ```suggestion
               var tables = new ArrayList<Table>(tableNames.size());
   
               try {
                   for (var fut : tableFuts) {
                       var table = fut.get();
   ```
   ```suggestion
              Arrays.stream(tableFuts).map(CompletableFuture::getNow(null)).filter(f -> f != null).collect(Collectors.toList())
   ```




-- 
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] ptupitsyn merged pull request #253: IGNITE-15218 Add async methods to IgniteTables

Posted by GitBox <gi...@apache.org>.
ptupitsyn merged pull request #253:
URL: https://github.com/apache/ignite-3/pull/253


   


-- 
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