You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "symious (via GitHub)" <gi...@apache.org> on 2023/12/13 01:58:47 UTC
[PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
symious opened a new pull request, #989:
URL: https://github.com/apache/ratis/pull/989
## What changes were proposed in this pull request?
In "https://github.com/apache/ratis/blob/9bd82aa2aa125a749b9357f208dd66533f43374c/ratis-common/src/main/java/org/apache/ratis/util/SlidingWindow.java#L363", the else block should not be reached, since only the first request is send out, all other request are still in the "delayedRequests", we can not receive out-of-order replies if "firstReplied" is not set.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1968
Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
No tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1853295835
Yes, seems the variables are reset on exceptions. I will try to dive deeper, if needed, I'll close the PR then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1855007265
The `failAllAsyncRequests` may fail with a different exception (`AlreadyClosedException` may work, or we should add a new `AlreadyFailedException`). For that exception, do not `resetSlidingWindow`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1857600004
@szetszwo I think `failAllAsyncRequests` is not invoked here, the slidingWindow is reset, then CompletionException(e) is thrown and `failAllAsyncRequests` is skipped. (Correct me if I'm wrong.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1853261790
Since `testWithLoadAsync` fails, it probably needs the else. Sorry that the code was long time ago. I might not remember the details. It may be related to failover to another server (i.e. leader change).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1854629354
> The error seems caused by repeating resets on the slidingWindow, ...
Where is the code?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-4365077279
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #989: RATIS-1968. Remove unreached else block of trySendDelayed
URL: https://github.com/apache/ratis/pull/989
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1854999331
> repeating reset.
https://github.com/apache/ratis/blob/cedcd2ad4cd2da13230aaa0d15678e5aee9b0729/ratis-client/src/main/java/org/apache/ratis/client/impl/OrderedAsync.java#L257C61-L257C61
Steps:
1. Client send 100 async requests to server, here firstReplied is true
2. Leader changed, 100 requests replied with exceptions, trying to reset the slidingWindow.
3. Say the requests id are [100, 199], client receives "id100" and and reset the slidingWindow, firstReplied changed to false.
4. Client retry on "id100" and receives the reply, so set firstReplied to true.
5. Client then receives other requests' exception reply from Step2, then reset the slidingWindow again, but the connection is ok now, no need to reset again.
@szetszwo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1853511025
@szetszwo The error seems caused by repeating resets on the slidingWindow, could you help to check?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-1853342701
@symious , I recall now -- when there is a failover, there could possibly be multiple calls needed to be retried. Since the retires are async, some other calls can be retried before the first call.
In short, client calls are always in order but retries may be out of order.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-1968. Remove unreached else block of trySendDelayed [ratis]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #989:
URL: https://github.com/apache/ratis/pull/989#issuecomment-4149129494
This PR has been marked as stale due to 60 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in ~30 days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org