[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20180 ) Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. [subprocess] KUDU-3489: Support reading large messages through pipes This patch enables the subprocess server to be able to read messages larger than 1MB which was otherwise flaky by reading the input stream messages until we encounter EOF. This issue is noticed when large sized requests are made to the subprocess server and it fails in receiving the complete messages. In addition made a small log change to MessageIO.java to display the exception message correctly. Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Reviewed-on: http://gerrit.cloudera.org:8080/20180 Reviewed-by: Alexey Serbin Tested-by: Abhishek Chennaka --- M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java M src/kudu/subprocess/subprocess_server-test.cc 4 files changed, 81 insertions(+), 29 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Abhishek Chennaka: Verified -- 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: merged Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 7 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka has removed a vote on this change. Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. Removed Verified-1 by Kudu Jenkins (120) -- 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: deleteVote Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 6 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka 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 6: Verified+1 Unrelated failure -- 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: 6 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 13 Jul 2023 05:00:33 + Gerrit-HasComments: No
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
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 6: Code-Review+2 -- 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: 6 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 13 Jul 2023 02:29:31 + Gerrit-HasComments: No
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka 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 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20180/5/src/kudu/subprocess/subprocess_server-test.cc
File src/kudu/subprocess/subprocess_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/20180/5/src/kudu/subprocess/subprocess_server-test.cc@327
PS5, Line 327: threads.reserve(kNumThreads);
: for (int i = 0; i < kNumThreads; i++) {
> nit: wrong indent
Done
--
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: 5
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 13 Jul 2023 01:56:32 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20180 to look at the new patch set (#6). Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. [subprocess] KUDU-3489: Support reading large messages through pipes This patch enables the subprocess server to be able to read messages larger than 1MB which was otherwise flaky by reading the input stream messages until we encounter EOF. This issue is noticed when large sized requests are made to the subprocess server and it fails in receiving the complete messages. In addition made a small log change to MessageIO.java to display the exception message correctly. Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c --- M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java M src/kudu/subprocess/subprocess_server-test.cc 4 files changed, 81 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/20180/6 -- 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: newpatchset Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 6 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
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 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20180/5/src/kudu/subprocess/subprocess_server-test.cc
File src/kudu/subprocess/subprocess_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/20180/5/src/kudu/subprocess/subprocess_server-test.cc@327
PS5, Line 327: threads.reserve(kNumThreads);
: for (int i = 0; i < kNumThreads; i++) {
nit: wrong indent
--
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: 5
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 13 Jul 2023 00:09:17 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
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 5: Code-Review+2 Thank you for the fix. BTW, have you looked at the C++ implementation of the Subprocess protocol w.r.t. the proper way of reading in the data? -- 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: 5 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 13 Jul 2023 00:03:42 + Gerrit-HasComments: No
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20180 to look at the new patch set (#5). Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. [subprocess] KUDU-3489: Support reading large messages through pipes This patch enables the subprocess server to be able to read messages larger than 1MB which was otherwise flaky. This issue is noticed when large sized requests are made to the subprocess server and it fails in receiving the complete messages. In addition made a small log change to MessageIO.java to display the exception message correctly. Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c --- M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java M src/kudu/subprocess/subprocess_server-test.cc 4 files changed, 82 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/20180/5 -- 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: newpatchset Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 5 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka 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 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20180/4/src/kudu/subprocess/subprocess_server-test.cc
File src/kudu/subprocess/subprocess_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/20180/4/src/kudu/subprocess/subprocess_server-test.cc@328
PS4, Line 328: threads.emplace_back([&, i] {
> warning: 'emplace_back' is called inside a loop; consider pre-allocating th
Done
--
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: 4
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 12 Jul 2023 23:47:26 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20180 to look at the new patch set (#4). Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. [subprocess] KUDU-3489: Support reading large messages through pipes This patch enables the subprocess server to be able to read messages larger than 1MB which was otherwise flaky. This issue is noticed when large sized requests are made to the subprocess server and it fails in receiving the complete messages. In addition made a small log change to MessageIO.java to display the exception message correctly. Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c --- M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java M src/kudu/subprocess/subprocess_server-test.cc 4 files changed, 80 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/20180/4 -- 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: newpatchset Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
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 3: Something is wrong, org.apache.kudu.subprocess.echo.TestEchoSubprocess has failed with PS3 in each build configuration (DEBUG, RELEASE, etc.): http://dist-test.cloudera.org/job?job_id=jenkins-slave.1689142881.508068 http://dist-test.cloudera.org/job?job_id=jenkins-slave.1689143265.2782414 -- 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: 3 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 12 Jul 2023 17:22:41 + Gerrit-HasComments: No
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka 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 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/20180/2/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/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@96
PS2, Line 96: Preconditions.checkNotNull(bytes);
: int totalRead = in.read(by
> Why not to rewrite this using do { ... } while (...) to logically unify thi
Done
http://gerrit.cloudera.org:8080/#/c/20180/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@101
PS2, Line 101: break;
: }
: t
> From the documentation of the BufferedInputStream.read() method:
Yes, as discussed removed the read == 0 condition and added a comment on the
behavior.
http://gerrit.cloudera.org:8080/#/c/20180/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@131
PS2, Line 131: }
: rem -= r
> ditto
Same as above
--
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: 3
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 12 Jul 2023 16:08:31 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Hello Tidy Bot, Alexey Serbin, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20180 to look at the new patch set (#3). Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. [subprocess] KUDU-3489: Support reading large messages through pipes This patch enables the subprocess server to be able to read messages larger than 1MB which was otherwise flaky. This issue is noticed when large sized requests are made to the subprocess server and it fails in receiving the complete messages. In addition made a small log change to MessageIO.java to display the exception message correctly. Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c --- M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java M src/kudu/subprocess/subprocess_server-test.cc 3 files changed, 78 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/20180/3 -- 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: newpatchset Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 3 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
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 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/20180/2/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/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@96
PS2, Line 96: int totalRead = in.read(bytes, 0, size);
: while (totalRead < size) {
Why not to rewrite this using do { ... } while (...) to logically unify this
implementation with the updated implementation of the doReadAndDiscard() method
below?
http://gerrit.cloudera.org:8080/#/c/20180/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@101
PS2, Line 101: } else if (read == 0) {
: break;
: }
>From the documentation of the BufferedInputStream.read() method:
--
This method implements the general contract of the corresponding read method of
the InputStream class. As an additional convenience, it attempts to read as
many bytes as possible by repeatedly invoking the read method of the underlying
stream. This iterated read continues until one of the following conditions
becomes true:
* The specified number of bytes have been read,
* The read method of the underlying stream returns -1, indicating
end-of-file, or
* The available method of the underlying stream returns zero, indicating that
further input requests would block.
--
Could it happen that the call at line 96 or 98 read out some non-empty chunk of
data up to the 'would block' condition on the underlying stream, while not all
the requested data has been actually read out? If so, then next cycle the call
at line 98 should have returned 0, and there would be an exception raised below
instead of calling BufferedInputStream.read() again that would block awaiting
for the next chunk of data to be read from the input stream?
http://gerrit.cloudera.org:8080/#/c/20180/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java@131
PS2, Line 131: } else if (read == 0) {
: break;
ditto
--
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: 2
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 11 Jul 2023 21:46:38 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20180 to look at the new patch set (#2). Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. [subprocess] KUDU-3489: Support reading large messages through pipes This patch enables the subprocess server to be able to read messages larger than 1MB which was otherwise flaky. This issue is noticed when large sized requests are made to the subprocess server and it fails in receiving the complete messages. In addition made a small log change to MessageIO.java to display the exception message correctly. Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c --- M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/TestMessageIO.java M src/kudu/subprocess/subprocess_server-test.cc 4 files changed, 72 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/20180/2 -- 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: newpatchset Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 2 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka 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
Done
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 'of
Yes, that makes sense. For some reason I thought it was an offset from which we
read data from, in the pipe.
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
Yes, true. It should be 0.
The same issue is seen here as well, if we don't "read" the expected size
("toRead") of the data while discarding the oversized message.
I just modified the loop to keep reading until we cannot read anymore and then
checking if we have read the expected amount of data.
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
Done
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
Done.
--
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
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 11 Jul 2023 00:04:40 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
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
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 10 Jul 2023 20:56:08 +
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] KUDU-3489: Support reading large messages through pipes
Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20180 Change subject: [subprocess] KUDU-3489: Support reading large messages through pipes .. [subprocess] KUDU-3489: Support reading large messages through pipes This patch enables the subprocess server to be able to read messages larger than 1MB which was otherwise flaky. This issue is noticed when large sized requests are made to the subprocess server and it fails in receiving the complete messages. In addition made a small log change to MessageIO.java to display the excpetion message correctly. Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c --- M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageIO.java M java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageReader.java M src/kudu/subprocess/subprocess_server-test.cc 3 files changed, 64 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/20180/1 -- 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: newchange Gerrit-Change-Id: I6523fdaaca19ee089dbac52a7dedec8847926a6c Gerrit-Change-Number: 20180 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka
