[GitHub] [storm] Ethanlm opened a new pull request #3282: [STORM-3649] fix logic error regarding storm.supervisor.medium.memory.grace.period.ms

[GitBox <gi...@apache.org>]

Ethanlm opened a new pull request #3282:
URL: https://github.com/apache/storm/pull/3282


   ## What is the purpose of the change
   
   Fix then bug that when supervisor medium.memory case happens, the logic about grace period is wrong. The code related is: 
   https://github.com/apache/storm/blob/2.2.x-branch/storm-server/src/main/java/org/apache/storm/daemon/supervisor/BasicContainer.java#L758
   ```
   if (memoryLimitExceededStart < 0) {
       memoryLimitExceededStart = Time.currentTimeMillis();
   } else {
       long timeInViolation = Time.currentTimeMillis() - memoryLimitExceededStart;
       if (timeInViolation > mediumMemoryGracePeriodMs) {
           LOG.warn(
               "{} is using {} MB > memory limit {} MB for {} seconds",
               typeOfCheck,
               usageMb,
               memoryLimitMb,
               timeInViolation / 1000);
           return true;
       }
   }
   ```
   The problem here is `memoryLimitExceededStart` is initialized to 0. The initial value should be less than 0. So we will see something like this in the log:
   ```
   2020-06-08 20:39:18.277 o.a.s.d.s.BasicContainer SLOT_6707 [WARN] WORKER 9c16e81e-4936-4029-bcda-ceb5b74b8f42 is using 167 MB > memory limit 158 MB for 1591648758 seconds
   ```
   
   ## How was the change tested
   
   1. Set up a single node storm cluster on Rhel6 node.
   2. Carefully chose memory related configs to make sure the supervisor medium memory case happens
   3. Submitted a word-count topology to validate the fix.
   
   Then we can see something like this:
   
   ```
   2020-06-08 20:55:08.550 o.a.s.d.s.BasicContainer SLOT_6707 [WARN] WORKER 06c3b2e6-c775-4bfa-99f6-0402dfed63d8 is using 170 MB > memory limit 158 MB for 30 seconds
   ```


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

[GitHub] [storm] Ethanlm merged pull request #3282: [STORM-3649] fix logic error regarding storm.supervisor.medium.memory.grace.period.ms

[GitBox <gi...@apache.org>]

Ethanlm merged pull request #3282:
URL: https://github.com/apache/storm/pull/3282


   


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