[jira] [Commented] (RATIS-140) Raft client should reuse the gRPC stream for all async calls
[ https://issues.apache.org/jira/browse/RATIS-140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16296311#comment-16296311 ] Jing Zhao commented on RATIS-140: - Some comments so far (after discussion with Nicholas offline): # Maybe we should use composition instead of inheritance for Client and RequestMap? Letting Client extends RequestMap may not be that direct. # Currently the size limitation of the total number of outstanding requests is defined in RaftClientImpl. I think to have the size limitation inside of SlidingWindow may be better. # We may still need a better abstraction for SlidingWindow. Maybe we should make it a pure data structure and avoid actions like "submissionMethod" inside of it as a member field. This abstraction later can also be adopted by AppendStreamer and GrpcLogAppender. # Currently for gRPC module, this patch actually cuts the underlying grpc streamer into small pieces (each onNext call), and then builds a consecutive async RPC call model on top. To me this may not be clean. We may want to revisit our API choice later (sendAsync v.s. OutputStream) in a separate jira. I think we can fix some minor issues and commit the patch in this jira. Then we can create follow-on jiras to discuss the above #3 and #4 comments. > Raft client should reuse the gRPC stream for all async calls > > > Key: RATIS-140 > URL: https://issues.apache.org/jira/browse/RATIS-140 > Project: Ratis > Issue Type: Bug >Reporter: Tsz Wo Nicholas Sze >Assignee: Tsz Wo Nicholas Sze > Attachments: r140_20171123.patch, r140_20171124.patch, > r140_20171125.patch, r140_20171126.patch, r140_20171126b.patch, > r140_20171130.patch, r140_20171203.patch, r140_20171204.patch, > r140_20171206.patch, r140_20171210.patch > > > Async client is being added in RATIS-113. However, we found that the server > side (RaftClientProtocolService) may see out-of-order grpc messages even if > all messages are sent by the same client. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RATIS-171) The FileStore tests fail with NullPointerException
[
https://issues.apache.org/jira/browse/RATIS-171?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16294806#comment-16294806
]
Tsz Wo Nicholas Sze commented on RATIS-171:
---
{code}
Running org.apache.ratis.examples.filestore.TestFileStoreWithNetty
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 16.348 sec - in
org.apache.ratis.examples.filestore.TestFileStoreWithNetty
Running org.apache.ratis.examples.filestore.TestFileStoreWithGrpc
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 24.31 sec - in
org.apache.ratis.examples.filestore.TestFileStoreWithGrpc
{code}
Just have tested the patch manually. Both FileStore tests passed.
+1 patch looks good.
> The FileStore tests fail with NullPointerException
> --
>
> Key: RATIS-171
> URL: https://issues.apache.org/jira/browse/RATIS-171
> Project: Ratis
> Issue Type: Bug
>Reporter: Tsz Wo Nicholas Sze
>Assignee: Kit Hui
> Attachments: r171_20171218.patch
>
>
> In the RaftLogWorker.WriteLog constructor, stateMachineFuture can be null
> since stateMachine.writeStateMachineData(entry) may return null. In such
> case, it throws NullPointerException when creating the combined future.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (RATIS-171) The FileStore tests fail with NullPointerException
[
https://issues.apache.org/jira/browse/RATIS-171?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16294764#comment-16294764
]
Hadoop QA commented on RATIS-171:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
10s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
1s{color} | {color:blue} Findbugs executables are not available. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
59s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
48s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
16s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
30s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
48s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
48s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 15s{color} | {color:orange} root: The patch generated 3 new + 82 unchanged -
1 fixed = 85 total (was 83) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
30s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 10m
6s{color} | {color:green} root in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
8s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 15m 37s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/ratis:date2017-12-18
|
| JIRA Issue | RATIS-171 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12902615/r171_20171218.patch |
| Optional Tests | asflicense javac javadoc unit findbugs checkstyle
compile |
| uname | Linux 36f8c60a8f5d 3.13.0-135-generic #184-Ubuntu SMP Wed Oct 18
11:55:51 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality |
/home/jenkins/jenkins-slave/workspace/PreCommit-RATIS-Build/yetus-personality.sh
|
| git revision | master / 99e409b |
| Default Java | 1.8.0_151 |
| checkstyle |
https://builds.apache.org/job/PreCommit-RATIS-Build/70/artifact/out/diff-checkstyle-root.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-RATIS-Build/70/testReport/ |
| modules | C: ratis-server U: ratis-server |
| Console output |
https://builds.apache.org/job/PreCommit-RATIS-Build/70/console |
| Powered by | Apache Yetus 0.5.0 http://yetus.apache.org |
This message was automatically generated.
> The FileStore tests fail with NullPointerException
> --
>
> Key: RATIS-171
> URL: https://issues.apache.org/jira/browse/RATIS-171
> Project: Ratis
> Issue Type: Bug
>Reporter: Tsz Wo Nicholas Sze
>Assignee: Kit Hui
> Attachments: r171_20171218.patch
>
>
> In the RaftLogWorker.WriteLog constructor, stateMachineFuture can be null
> since stateMachine.writeStateMachineData(entry) may return null. In such
> case, it throws NullPointerException when creating the combined future.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Updated] (RATIS-171) The FileStore tests fail with NullPointerException
[ https://issues.apache.org/jira/browse/RATIS-171?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kit Hui updated RATIS-171: -- Attachment: r171_20171218.patch Move the assignment for this.combine after handling the assignment for this.stateMachineFuture. > The FileStore tests fail with NullPointerException > -- > > Key: RATIS-171 > URL: https://issues.apache.org/jira/browse/RATIS-171 > Project: Ratis > Issue Type: Bug >Reporter: Tsz Wo Nicholas Sze >Assignee: Kit Hui > Attachments: r171_20171218.patch > > > In the RaftLogWorker.WriteLog constructor, stateMachineFuture can be null > since stateMachine.writeStateMachineData(entry) may return null. In such > case, it throws NullPointerException when creating the combined future. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
