Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20180 )
Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/20180/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20180/1//COMMIT_MSG@15 PS1, Line 15: excpetion exception http://gerrit.cloudera.org:8080/#/c/20180/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java: http://gerrit.cloudera.org:8080/#/c/20180/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@96 PS1, Line 96: byte[] b = new byte[size] Instead of allocating an extra memory buffer, is it possible to use the 'offset' parameter of the read() call below for partial reads? http://gerrit.cloudera.org:8080/#/c/20180/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@140 PS1, Line 140: if (rem > 4096) { : throw new IOException( : String.format("unable to read next chunk of oversized message (%d bytes), " + : "expected %d bytes but read %d bytes", size, size, size - rem)); : } What if that was less that 4096? Shouldn't it be an exceptional situation as well? Overall, why to change the way how this method reads out the data? What are you trying to fix here? http://gerrit.cloudera.org:8080/#/c/20180/1/src/kudu/subprocess/subprocess_server-test.cc File src/kudu/subprocess/subprocess_server-test.cc: http://gerrit.cloudera.org:8080/#/c/20180/1/src/kudu/subprocess/subprocess_server-test.cc@426 PS1, Line 426: nit: remove the extra empty line http://gerrit.cloudera.org:8080/#/c/20180/1/src/kudu/subprocess/subprocess_server-test.cc@432 PS1, Line 432: the current limit Does it make sense to set FLAGS_subprocess_max_message_size_bytes to 2x (or even 3x) higher value that its default to see how it works if going beyond the default setting? -- To view, visit http://gerrit.cloudera.org:8080/20180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 10 Jul 2023 20:56:08 +0000 Gerrit-HasComments: Yes
