Yuqi Du 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 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/19949/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19949/3//COMMIT_MSG@21 PS3, Line 21: the a http://gerrit.cloudera.org:8080/#/c/19949/3//COMMIT_MSG@20 PS3, Line 20: This patch will validate the schema of multiple rows which belong : to the same tablet. If the schema is different, it puts them into : the different groups as different batches. Great! This bug is a serious problem. This way seems ok and there is something worried. I comment this at other comment. I reviewed what you say, and provide another way to fix this bug. You can review and considering it whether it is ok, like this: ``` // OperationsEncoder in Operation.java. private void init(List<Operation> operations) { int rowsSize = 0; int indirectSize = 0; for (Operation op : operations) { Schema schema = op.table.getSchema(); final int columnBitSetSize = Bytes.getBitSetSize(schema.getColumnCount()); int sizePerRow = 1 /* for the op type */ + schema.getRowSize() + columnBitSetSize; if (schema.hasNullableColumns()) { // nullsBitSet is the same size as the columnBitSet sizePerRow += columnBitSetSize; } rowsSize += sizePerRow; indirectSize += schema.getVarLengthColumnCount(); } this.rows = ByteBuffer.allocate(rowsSize).order(ByteOrder.LITTLE_ENDIAN); this.indirect = new ArrayList<>(indirectSize); } ``` http://gerrit.cloudera.org:8080/#/c/19949/3/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/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@371 PS3, Line 371: Extral I guess you mean 'Extra' ? http://gerrit.cloudera.org:8080/#/c/19949/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@372 PS3, Line 372: extral_batches extra_batches http://gerrit.cloudera.org:8080/#/c/19949/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java@419 PS3, Line 419: extral_batches every op different from batch.operations.get(0) need a write rpc? It seems not good. If this op can merge into an extra batch, it may be better. By the way, I am not sure a potential problem because I am not care about txn in kudu. I worry about this fixing way may cause another problem, there is a txn Id. This fixing maybe split a txn into 2 or more rpcs. -- 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: 3 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Fri, 02 Jun 2023 04:14:21 +0000 Gerrit-HasComments: Yes
