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 2022/01/26 18:20:14 UTC

[GitHub] [helix] desaikomal opened a new pull request #1944: Fix for - Stale message redundant logs #1940

desaikomal opened a new pull request #1944:
URL: https://github.com/apache/helix/pull/1944

Do not generate unnecessary stale messages for irrelevant resource of instances.
We generate all stale messages for instance ignoring resource context.

### Issues

- [ ] My PR addresses the following Helix issues and references them in the PR description:

Resolves #1940

### Description

- [ ] Here are some details about my PR, including screenshots of any UI changes:

The change adds validation if the resource name is same as for which message needs. If they are different, ignore it.

### Tests

- [ ] The following tests are written for this issue:

(List the names of added unit/integration tests)

- The following is the result of the "mvn test" command on the appropriate module:

(If CI test fails due to known issue, please specify the issue and test PR locally. Then copy & paste the result of "mvn test" to here.)

[INFO] Results:
[INFO]
[INFO] Tests run: 1287, Failures: 0, Errors: 0, Skipped: 0
[INFO]

### Changes that Break Backward Compatibility (Optional)

- My PR contains changes that break backward compatibility or previous assumptions for certain methods or API. They include:

My change is a bug fix and does not impact any API/backward compatibility.

### 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.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

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] desaikomal closed pull request #1944: Fix for - Stale message redundant logs #1940

Posted by GitBox <gi...@apache.org>.

desaikomal closed pull request #1944:
URL: https://github.com/apache/helix/pull/1944


   


-- 
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: reviews-unsubscribe@helix.apache.org

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 #1944: Fix for - Stale message redundant logs #1940

Posted by GitBox <gi...@apache.org>.

NealSun96 commented on a change in pull request #1944:
URL: https://github.com/apache/helix/pull/1944#discussion_r792954535



##########
File path: helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java
##########
@@ -200,9 +200,10 @@ private void generateMessage(final Resource resource, final BaseControllerDataPr
         }
 
         for (Message staleMessage : staleMessages) {
-          if (System.currentTimeMillis() - currentStateOutput
+          if ((System.currentTimeMillis() - currentStateOutput
               .getEndTime(resourceName, partition, instanceName)
-              > DEFAULT_OBSELETE_MSG_PURGE_DELAY) {
+              > DEFAULT_OBSELETE_MSG_PURGE_DELAY)
+              && staleMessage.getResourceName().equals(resourceName)) {

Review comment:
       Since this logic is under per-resource, per-partition and per-instance condition, we should also check whether the staleMessage's corresponding partition matches the current `partition`, on top of checking resource. Otherwise, if the instance has multiple partitions of this resource, this log will still be duplicate. 




-- 
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: reviews-unsubscribe@helix.apache.org

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