Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19949 )
Change subject: [KUDU-3483] Fixbug of auto flush data when table schema changed ...................................................................... Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/19949/10/java/kudu-client/src/main/java/org/apache/kudu/Schema.java File java/kudu-client/src/main/java/org/apache/kudu/Schema.java: http://gerrit.cloudera.org:8080/#/c/19949/10/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@464 PS10, Line 464: equals Howa about split this modification to another patch (of course add some tests as well)? http://gerrit.cloudera.org:8080/#/c/19949/10/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java: http://gerrit.cloudera.org:8080/#/c/19949/10/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@416 PS10, Line 416: !batch.operations.get(0).table.getSchema().equals(operation.table.getSchema()) It's a waste if the schema is 1, 2, 2, 2, 2, 2. The ops in schema 2 will not be batched. How about define the batches like: // Group the operations by tablet, the batches are separated by different schemas. Map<Slice, List<Batch>> last_batches; You can buffer the operations in a batch, push the buffered batch to the list and switch to a new one when found a different schema. http://gerrit.cloudera.org:8080/#/c/19949/10/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java: http://gerrit.cloudera.org:8080/#/c/19949/10/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@141 PS10, Line 141: KuduTable table = createTable(ImmutableList.of(), null, 1); Add comments to explain why create a single bucket. http://gerrit.cloudera.org:8080/#/c/19949/10/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@165 PS10, Line 165: KuduScanner scanner = client.newScannerBuilder(table).build(); : int rowCount = 0; : while (scanner.hasMoreRows()) { : RowResultIterator it = scanner.nextRows(); : while (it.hasNext()) { : it.next(); : ++rowCount; : } : } : assertEquals(2, rowCount); Wrap a function? you can reuse it below. -- To view, visit http://gerrit.cloudera.org:8080/19949 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6501962b32814d121f180b2942999c402d927db Gerrit-Change-Number: 19949 Gerrit-PatchSet: 10 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Fri, 16 Jun 2023 10:27:33 +0000 Gerrit-HasComments: Yes
