Wang Xixu 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 8:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/19949/7/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/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@346
PS7, Line 346: batches by tablet
> nit: The comments have to be updated.
Done


http://gerrit.cloudera.org:8080/#/c/19949/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@372
PS7, Line 372: Extra batches for operations which have different table schemas.
             :       // 'extraBatches' is used to st
> How about: 'extraBatches' is used to store batches of operations which have
Done


http://gerrit.cloudera.org:8080/#/c/19949/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@415
PS7, Line 415: se if
> nit: !batch
Done


http://gerrit.cloudera.org:8080/#/c/19949/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@423
PS7, Line 423:
> Should be:
Done


http://gerrit.cloudera.org:8080/#/c/19949/7/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/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@139
PS7, Line 139:
> I see you flush manually below, why set the mode as AUTO_FLUSH_BACKGROUND h
Every mode will cause the problem. It no needs to set the mode 
AUTO_FLUSH_BACKGROUND.


http://gerrit.cloudera.org:8080/#/c/19949/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@151
PS7, Line 151:    * @param end the exclusive end key
             :    */
             :
> Is it too special that upsert on the same row? It's enough to cover the cas
Done


http://gerrit.cloudera.org:8080/#/c/19949/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java@190
PS7, Line 190:       assertEquals(String.format("row errors: %s",
> Also check we can read the rows correctly.
Done



--
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: 8
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: Thu, 15 Jun 2023 04:21:16 +0000
Gerrit-HasComments: Yes

Reply via email to