You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/10/12 23:55:32 UTC
[GitHub] [helix] NealSun96 opened a new pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
NealSun96 opened a new pull request #1460:
URL: https://github.com/apache/helix/pull/1460
### Issues
- [x] My PR addresses the following Helix issues and references them in the PR description:
Fixes #1459
### Description
- [x] Here are some details about my PR, including screenshots of any UI changes:
If a reply message doesn't have a corresponding callback, AsyncCallbackService will raise an exception when creating a handler for it. Since stale reply messages may happen during participant reconnects, some customers prefer handling them without raising an exception.
This PR changes behavior such that handlers are allowed to be created with non-matched correlation ids, and no callback will be triggered when they are handled. This PR also improves an old test case which didn't test certain conditions correctly.
### Tests
- [x] The following tests are written for this issue:
```
[ERROR] Tests run: 1213, Failures: 1, Errors: 0, Skipped: 1, Time elapsed: 4,830.531 s <<< FAILURE! - in TestSuite
[ERROR] testPeriodicRefresh(org.apache.helix.integration.spectator.TestRoutingTableProviderPeriodicRefresh) Time elapsed: 2.031 s <<< FAILURE!
java.lang.AssertionError: expected:<4> but was:<3>
at org.apache.helix.integration.spectator.TestRoutingTableProviderPeriodicRefresh.testPeriodicRefresh(TestRoutingTableProviderPeriodicRefresh.java:214)
[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] TestRoutingTableProviderPeriodicRefresh.testPeriodicRefresh:214 expected:<4> but was:<3>
[INFO]
[ERROR] Tests run: 1213, Failures: 1, Errors: 0, Skipped: 1
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:20 h
[INFO] Finished at: 2020-10-12T16:36:07-07:00
[INFO] ------------------------------------------------------------------------
```
### Documentation (Optional)
- In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)
### Commits
- My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Code Quality
- My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504343539
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -62,15 +62,6 @@ void verifyMessage(Message message) {
throw new HelixException(errorMsg);
}
Review comment:
Alternatively, if the exception is not graceful, we can return null when createHandler() is triggered. This is much better than returning a handler that does nothing (but still consume system resources).
Of course, the callers of createHandler() need to be improved to tolerate the null return value.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] zhangmeng916 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
zhangmeng916 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504121634
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" + _correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" + _correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
A question regarding the description, if the error is just logged here, meaning the users won't really know there is a mismatch. If the callback is not done, they won't be able to distinguish between this error and other errors. Is this the expected behavior?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] kaisun2000 commented on pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
kaisun2000 commented on pull request #1460:
URL: https://github.com/apache/helix/pull/1460#issuecomment-708626265
As discussed, the main concern is that current way, additional thread created.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] zhangmeng916 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
zhangmeng916 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504172942
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" + _correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" + _correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
All i would like to ask is whether removing the previous thrown exception will cause any information lost. If it's ok, then I'm fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on a change in pull request #1460: Silence Stacktrace from Stale Reply Messages
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r508101772
##########
File path: helix-common/src/main/java/org/apache/helix/HelixException.java
##########
@@ -30,6 +30,16 @@ public HelixException(String message) {
super(message);
}
+ /**
+ * Create a HelixException that can optionally turn off stack trace. Its other characteristics are
+ * the same as a HelixException with a message.
+ * @param message the detail message
+ * @param writableStackTrace whether or not the stack trace should be writable
+ */
+ public HelixException(String message, boolean writableStackTrace) {
Review comment:
Yes sir
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang edited a comment on pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
jiajunwang edited a comment on pull request #1460:
URL: https://github.com/apache/helix/pull/1460#issuecomment-708092867
Hold on, please.
I'm trying to understand why the Exception is an issue here. If the reply message is stale, meaning we don't have enough information to handle it, fail the handler creation op seems to be a reasonable solution.
If the operation is not failed out, then the handling will be finished with a successful result. It could be misleading.
This basically means bad clients can send invalid messages response to the host and DDoS it. Since the host cannot reject any messages, it will allocate many resources. And this is concerning.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504171428
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" + _correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" + _correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
I'm not sure I follow: this is not a "try" block, this is an "if" block. There's no exception handling here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] alirezazamani merged pull request #1460: Silence Stacktrace from Stale Reply Messages
Posted by GitBox <gi...@apache.org>.
alirezazamani merged pull request #1460:
URL: https://github.com/apache/helix/pull/1460
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] zhangmeng916 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
zhangmeng916 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504280876
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" + _correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" + _correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
Yep, that's what I meant. I think it's not a big concern.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504128305
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" + _correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" + _correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
You mean they wouldn't be able to distinguish "stale messages" vs "messages that don't have callbacks set up correctly"? I think these cases are logically the same and can't be easily distinguished code-level. If we are loosening up on one case, it's going to affect the other.
On the other hand, callback registration is done in our code and naturally there should be a callback before sending messages that require replies. I imagine the previous check was there for cautionary purpose, not that messages are commonly created with incorrect correlation ids.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504279775
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" + _correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" + _correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
I see what you mean now: you're saying in both cases, callback.isDone() == false, and it can't be distinguished whether the callback failed or there's no callback registered.
If the callback fails due to some exception like you said, the task result will reflect that (setException(e), setInterrupted(true)). It's more difficult to tell the "callback not registered" case, but since customers don't want exceptions raised, there isn't any better way here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504884937
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -62,15 +62,6 @@ void verifyMessage(Message message) {
throw new HelixException(errorMsg);
}
Review comment:
You brought up a very good point, although for our scenario, ZK read probably fails before an effective DOS. I understand this reminds you of some TCP DOS. :)
If you could check out `HelixTaskExecutor`, at around line 900, you can see how we handle null return value for `createHandler()`. The current approach is to skip the message without cleaning it up. I could explore on changing this current approach (because we need to clean up those reply messages), but such a change will impact other message handlings as well (such as state transition messages).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on pull request #1460: Silence Stacktrace from Stale Reply Messages
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on pull request #1460:
URL: https://github.com/apache/helix/pull/1460#issuecomment-713070763
This PR is ready to be merged, approved by @kaisun2000
Final commit message:
## Silence Stacktrace from Stale Reply Messages ##
This PR adds a less verbose (no stacktrace) HelixException and raises it for stale reply message cases.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] zhangmeng916 commented on a change in pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
zhangmeng916 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r504152792
##########
File path: helix-core/src/main/java/org/apache/helix/messaging/handling/AsyncCallbackService.java
##########
@@ -113,12 +104,19 @@ public HelixTaskResult handleMessage() throws InterruptedException {
+ _correlationId);
AsyncCallback callback = _callbackMap.get(_correlationId);
- synchronized (callback) {
- callback.onReply(_message);
- if (callback.isDone()) {
- _logger.info("Removing finished callback, correlationid:" + _correlationId);
- _callbackMap.remove(_correlationId);
+ if (callback != null) {
+ synchronized (callback) {
+ callback.onReply(_message);
+ if (callback.isDone()) {
+ _logger.info("Removing finished callback, correlationid:" + _correlationId);
+ _callbackMap.remove(_correlationId);
+ }
}
+ } else {
+ String msg = "Message " + _message.getMsgId()
+ + " does not have correponding callback. Probably timed out already. Correlation id: "
+ + _correlationId;
+ _logger.warn(msg);
Review comment:
Actually I meant this missing ID exception and exceptions in the later functions in onReply. Do we need to consider distinguishing between them? like in your test `Assert.assertFalse(callback.isDone())`. Any other errors may happen in middle?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] kaisun2000 commented on a change in pull request #1460: Silence Stacktrace from Stale Reply Messages
Posted by GitBox <gi...@apache.org>.
kaisun2000 commented on a change in pull request #1460:
URL: https://github.com/apache/helix/pull/1460#discussion_r508079268
##########
File path: helix-common/src/main/java/org/apache/helix/HelixException.java
##########
@@ -30,6 +30,16 @@ public HelixException(String message) {
super(message);
}
+ /**
+ * Create a HelixException that can optionally turn off stack trace. Its other characteristics are
+ * the same as a HelixException with a message.
+ * @param message the detail message
+ * @param writableStackTrace whether or not the stack trace should be writable
+ */
+ public HelixException(String message, boolean writableStackTrace) {
Review comment:
So RuntimeException can be configured not to have stack trace out.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] NealSun96 commented on pull request #1460: Silence Stacktrace from Stale Reply Messages
Posted by GitBox <gi...@apache.org>.
NealSun96 commented on pull request #1460:
URL: https://github.com/apache/helix/pull/1460#issuecomment-709625584
Update: after group discussion, changed the strategy to silencing the exception stacktrace instead of changing behavior. This still satisfies customer needs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on pull request #1460: Allow Stale Reply Messages to Pass without Exceptions
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on pull request #1460:
URL: https://github.com/apache/helix/pull/1460#issuecomment-708092867
Hold on, please.
I'm trying to understand why the Exception is an issue here. If the reply message is stale, meaning we don't have enough information to handle it, fail the handler creation op seems to be a reasonable solution.
If the operation is not failed out, then the handling will be finished with a successful result. It could be misleading.
This basically means bad clients can send an invalid message response to the host and DDoS it. Since the host cannot reject any messages, it will allocate many resources. And this is concerning.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org