Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-5547

NMLeveldbStateStore should be more tolerant of unknown keys

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.6.0
    • Fix Version/s: 2.9.0, 3.0.0-alpha4
    • Component/s: nodemanager
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Whenever new keys are added to the NM state store it will break rolling downgrades because the code will throw if it encounters an unrecognized key. If instead it skipped unrecognized keys it could be simpler to continue supporting rolling downgrades. We need to define the semantics of unrecognized keys when containers and apps are cleaned up, e.g.: we may want to delete all keys underneath an app or container directory when it is being removed from the state store to prevent leaking unrecognized keys.

      1. YARN-5547.01.patch
        5 kB
        Ajith S
      2. YARN-5547.02.patch
        10 kB
        Ajith S
      3. YARN-5547.03.patch
        10 kB
        Ajith S
      4. YARN-5547.04.patch
        11 kB
        Ajith S
      5. YARN-5547.05.branch-2.patch
        11 kB
        Ajith S
      6. YARN-5547.05.patch
        11 kB
        Ajith S

        Issue Links

          Activity

          Hide
          jlowe Jason Lowe added a comment -

          Also see the backwards-compatibility discussions in YARN-3998 and YARN-5049.

          Show
          jlowe Jason Lowe added a comment - Also see the backwards-compatibility discussions in YARN-3998 and YARN-5049 .
          Hide
          ajithshetty Ajith S added a comment -

          Thanks Jason Lowe. I would like to work on this. Incase you have already started working on this, please feel free to assign back

          Show
          ajithshetty Ajith S added a comment - Thanks Jason Lowe . I would like to work on this. Incase you have already started working on this, please feel free to assign back
          Hide
          ajithshetty Ajith S added a comment -

          Hi Jason Lowe

          I have attached the patch for handling the exception but i have a doubt about prevent leaking unrecognized keys. . can you please elaborate.?

          Show
          ajithshetty Ajith S added a comment - Hi Jason Lowe I have attached the patch for handling the exception but i have a doubt about prevent leaking unrecognized keys. . can you please elaborate.?
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for the patch!

          What I meant about the leak is a scenario like this:

          1. NM is running version V which introduced a new key K that is associated with containers.
          2. A container is running which causes K to be written to the state store
          3. User does a rolling downgrade to V-1. The code ignores unrecognized key K.
          4. The container completes and the container is removed from the state store. This only removes the container keys version V knows about, and K is not one of those keys.
          5. At this point K has been leaked in the state store.
          6. That leak will be permanent until a rolling upgrade to >= V. Even then K might not be cleaned up since all the other container state has been removed, probably interfering with the typical recovery flow for that key type.

          There are a couple of risks when cleaning up unrecognized keys. The old version may be removing the key too early in the lifecycle of that state such that if we do a rolling upgrade back to the version that works with those keys we've incorrectly destroyed the state. We probably need to think more about the ramifications of cleaning unrecognized keys and when we should or shouldn't do so. Appreciate any thoughts on this.

          The other risk is that doing this cleaning will add a place where the NM will read the state store as it scans for keys to remove, and previously it only ever wrote to the store after the initial recover on startup. Writes to leveldb are typically very fast, whereas reads could be much slower depending upon how much the database needs to be compacted and how many blocks are involved in the scan. This is likely a minor concern especially with the recent periodic full compaction to the store, but it will impact state store performance to some degree.

          As for the patch the changes will make the NM more tolerant of new container keys, but there are other places where unexpected keys will break the state store recovery. loadResourceTrackerState and loadUserLocalizedResources are some other places that should be updated and there are similar questions there as to what should be done about cleanup of unexpected keys.

          Show
          jlowe Jason Lowe added a comment - Thanks for the patch! What I meant about the leak is a scenario like this: NM is running version V which introduced a new key K that is associated with containers. A container is running which causes K to be written to the state store User does a rolling downgrade to V-1. The code ignores unrecognized key K. The container completes and the container is removed from the state store. This only removes the container keys version V knows about, and K is not one of those keys. At this point K has been leaked in the state store. That leak will be permanent until a rolling upgrade to >= V. Even then K might not be cleaned up since all the other container state has been removed, probably interfering with the typical recovery flow for that key type. There are a couple of risks when cleaning up unrecognized keys. The old version may be removing the key too early in the lifecycle of that state such that if we do a rolling upgrade back to the version that works with those keys we've incorrectly destroyed the state. We probably need to think more about the ramifications of cleaning unrecognized keys and when we should or shouldn't do so. Appreciate any thoughts on this. The other risk is that doing this cleaning will add a place where the NM will read the state store as it scans for keys to remove, and previously it only ever wrote to the store after the initial recover on startup. Writes to leveldb are typically very fast, whereas reads could be much slower depending upon how much the database needs to be compacted and how many blocks are involved in the scan. This is likely a minor concern especially with the recent periodic full compaction to the store, but it will impact state store performance to some degree. As for the patch the changes will make the NM more tolerant of new container keys, but there are other places where unexpected keys will break the state store recovery. loadResourceTrackerState and loadUserLocalizedResources are some other places that should be updated and there are similar questions there as to what should be done about cleanup of unexpected keys.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 36s trunk passed
          +1 compile 0m 25s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 26s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 41s trunk passed
          +1 javadoc 0m 17s trunk passed
          +1 mvninstall 0m 21s the patch passed
          +1 compile 0m 24s the patch passed
          +1 javac 0m 24s the patch passed
          -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 3 new + 19 unchanged - 0 fixed = 22 total (was 19)
          +1 mvnsite 0m 24s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          -1 whitespace 0m 0s The patch has 7 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 findbugs 0m 45s the patch passed
          +1 javadoc 0m 14s the patch passed
          +1 unit 13m 12s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          25m 49s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825646/YARN-5547.01.patch
          JIRA Issue YARN-5547
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c179b52ad91b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 4bd45f5
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12941/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/12941/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12941/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12941/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 36s trunk passed +1 compile 0m 25s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 26s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 41s trunk passed +1 javadoc 0m 17s trunk passed +1 mvninstall 0m 21s the patch passed +1 compile 0m 24s the patch passed +1 javac 0m 24s the patch passed -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 3 new + 19 unchanged - 0 fixed = 22 total (was 19) +1 mvnsite 0m 24s the patch passed +1 mvneclipse 0m 10s the patch passed -1 whitespace 0m 0s The patch has 7 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 0m 45s the patch passed +1 javadoc 0m 14s the patch passed +1 unit 13m 12s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 25m 49s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825646/YARN-5547.01.patch JIRA Issue YARN-5547 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c179b52ad91b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4bd45f5 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12941/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/12941/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12941/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/12941/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          Cancelling the patch until we can work through some more issues with ignoring of keys. As pointed out in YARN-5630 there are going to be some cases where we do not want the NM to come up if it isn't going to honor certain container keys it finds in the state store.

          Show
          jlowe Jason Lowe added a comment - Cancelling the patch until we can work through some more issues with ignoring of keys. As pointed out in YARN-5630 there are going to be some cases where we do not want the NM to come up if it isn't going to honor certain container keys it finds in the state store.
          Hide
          subru Subru Krishnan added a comment -

          +1 on the need for this.

          Jason Lowe, you are bringing up good points. I think if we mark keys as ignore-able (or not) explicitly whenever new ones are added, shouldn't we able to handle the scenarios? We should still be more deliberate when introducing mandatory keys but it's at least explicit during deployment.

          Show
          subru Subru Krishnan added a comment - +1 on the need for this. Jason Lowe , you are bringing up good points. I think if we mark keys as ignore-able (or not) explicitly whenever new ones are added, shouldn't we able to handle the scenarios? We should still be more deliberate when introducing mandatory keys but it's at least explicit during deployment.
          Hide
          jlowe Jason Lowe added a comment -

          That sounds like an excellent idea. If the old software could consult a table in the database that lists what keys are ignorable then it can fail for any unrecognized key that isn't in that list and safely ignore ones that are.

          Show
          jlowe Jason Lowe added a comment - That sounds like an excellent idea. If the old software could consult a table in the database that lists what keys are ignorable then it can fail for any unrecognized key that isn't in that list and safely ignore ones that are.
          Hide
          asuresh Arun Suresh added a comment -

          I just had an offline discussion with Chris Douglas. He was also suggesting a possible middle ground. Instead of having the entire NM go down because the store can't recognize a key like we do now, or skipping the specific key like this patch initially proposed and hence leave the container in a weird state... does it make sense to just skip that particular container entirely ? Thoughts ?

          Show
          asuresh Arun Suresh added a comment - I just had an offline discussion with Chris Douglas . He was also suggesting a possible middle ground. Instead of having the entire NM go down because the store can't recognize a key like we do now, or skipping the specific key like this patch initially proposed and hence leave the container in a weird state... does it make sense to just skip that particular container entirely ? Thoughts ?
          Hide
          jlowe Jason Lowe added a comment -

          Skipping the container entirely would be very bad. The NM would not recover it, so it would then stop reporting it in heartbeats and the RM would then think it is dead/lost, but the container is actually still running, unmonitored and unkillable by YARN.

          Show
          jlowe Jason Lowe added a comment - Skipping the container entirely would be very bad. The NM would not recover it, so it would then stop reporting it in heartbeats and the RM would then think it is dead/lost, but the container is actually still running, unmonitored and unkillable by YARN.
          Hide
          chris.douglas Chris Douglas added a comment -

          Skipping the container entirely would be very bad. The NM would not recover it, so it would then stop reporting it in heartbeats and the RM would then think it is dead/lost, but the container is actually still running, unmonitored and unkillable by YARN.

          Agreed. What we were discussing was making container recovery independent, so containers using unknown features are not recovered, but failed and killed. The base case should recover nothing- all containers should be killed and cleaned up- but the NM should always start. I'm not sure every feature is neatly classified in the mandatory/optional taxonomy, particularly since many will depend on the version of the client and RM. It seems simpler (and safer) to always kill/clean up containers using features the NM doesn't understand.

          Show
          chris.douglas Chris Douglas added a comment - Skipping the container entirely would be very bad. The NM would not recover it, so it would then stop reporting it in heartbeats and the RM would then think it is dead/lost, but the container is actually still running, unmonitored and unkillable by YARN. Agreed. What we were discussing was making container recovery independent, so containers using unknown features are not recovered, but failed and killed. The base case should recover nothing- all containers should be killed and cleaned up- but the NM should always start. I'm not sure every feature is neatly classified in the mandatory/optional taxonomy, particularly since many will depend on the version of the client and RM. It seems simpler (and safer) to always kill/clean up containers using features the NM doesn't understand.
          Hide
          jlowe Jason Lowe added a comment -

          Yes, having the ability to recover unknown keys that cannot be ignored by failing their corresponding containers would be a nice addition and work well with the approach of only storing those keys when absolutely necessary. Bonus points if there's the ability to indicate that even failing the container isn't sufficient to properly a particular unknown key. e.g.: the NM has to unregister with a service as part of the container failure or cleanup some other local space, etc. that the old software doesn't know how to do.

          We'd still need to address policies for keys encountered outside of the container key space, e.g.: application keys, deletion service keys, keys for entirely new top-level systems in the state store, etc.

          Show
          jlowe Jason Lowe added a comment - Yes, having the ability to recover unknown keys that cannot be ignored by failing their corresponding containers would be a nice addition and work well with the approach of only storing those keys when absolutely necessary. Bonus points if there's the ability to indicate that even failing the container isn't sufficient to properly a particular unknown key. e.g.: the NM has to unregister with a service as part of the container failure or cleanup some other local space, etc. that the old software doesn't know how to do. We'd still need to address policies for keys encountered outside of the container key space, e.g.: application keys, deletion service keys, keys for entirely new top-level systems in the state store, etc.
          Hide
          ajithshetty Ajith S added a comment -

          So we have two approaches discussed here
          1. Either skip container recovery - this will cause unmonitered containers
          2. Container killed/failed

          I am ok with second approach, but as per Jason Lowe The NM has to unregister with a service as part of the container failure i don't see any solution for such scenario. If this case we can handle separately, i can update patch based on second approach

          Show
          ajithshetty Ajith S added a comment - So we have two approaches discussed here 1. Either skip container recovery - this will cause unmonitered containers 2. Container killed/failed I am ok with second approach, but as per Jason Lowe The NM has to unregister with a service as part of the container failure i don't see any solution for such scenario. If this case we can handle separately, i can update patch based on second approach
          Hide
          ajithshetty Ajith S added a comment -

          As per offline discussion with NGarla_Unused and Varun Saxena for If the old software could consult a table in the database that lists what keys are ignorable then it can fail for any unrecognized key that isn't in that list and safely ignore ones that are we can add a suffix for the keys if they are ignoreable so that even lower version will know if keys can be skipped safely

          Show
          ajithshetty Ajith S added a comment - As per offline discussion with NGarla_Unused and Varun Saxena for If the old software could consult a table in the database that lists what keys are ignorable then it can fail for any unrecognized key that isn't in that list and safely ignore ones that are we can add a suffix for the keys if they are ignoreable so that even lower version will know if keys can be skipped safely
          Hide
          jlowe Jason Lowe added a comment -

          To be clear, the skipping containers during recovery is never the right thing to do, so that's not really a valid option.

          As I understand it, the current proposal is this:

          • Add a new table to the state store that will contain a list of container keys that need compatibility processing when performing rolling downgrades
          • Each key in that table will have a descriptor associated with it that will indicate how the recovery of the corresponding container needs to be handled. Options include:
            • Killing the corresponding container
            • Removing the key and recovering the container normally
          • Any unrecognized container key that is not described in the table will cause the corresponding container to be killed during recovery.

          We don't have to implement the entire thing in this JIRA. We could do the unrecognized=kill implementation first then add the table of keys feature in a subsequent JIRA.

          Show
          jlowe Jason Lowe added a comment - To be clear, the skipping containers during recovery is never the right thing to do, so that's not really a valid option. As I understand it, the current proposal is this: Add a new table to the state store that will contain a list of container keys that need compatibility processing when performing rolling downgrades Each key in that table will have a descriptor associated with it that will indicate how the recovery of the corresponding container needs to be handled. Options include: Killing the corresponding container Removing the key and recovering the container normally Any unrecognized container key that is not described in the table will cause the corresponding container to be killed during recovery. We don't have to implement the entire thing in this JIRA. We could do the unrecognized=kill implementation first then add the table of keys feature in a subsequent JIRA.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Jason Lowe for conclusion,
          Agree having a table is better than having a suffix to a key to identify characteristics about the key and +1 for "unrecognized=kill implementation" as focus of this jira

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Jason Lowe for conclusion, Agree having a table is better than having a suffix to a key to identify characteristics about the key and +1 for "unrecognized=kill implementation" as focus of this jira
          Hide
          ajithshetty Ajith S added a comment -

          Please review

          Show
          ajithshetty Ajith S added a comment - Please review
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Ajith S,
          Patch does not seem to apply now, please rebase the patch and submit the path for jenkins to kick in.
          Overall approach seems to be fine, except for these few nits

          1. ContainerManagerImpl, ln 377: "Container killed after recovery.", may be better message like "Due to invalid StateStore info container was killed during recovery""
          2. ContainerManagerImpl, ln 386 : new line not req.
          3. NMLeveldbStateStoreService, ln 268 : requires logging(preferably warn) that the container <containerId> will be killed because of the unknown key <key> during recovery.
          4. NMLeveldbStateStoreService, ln 1231 : seems to be already handled by a other patch, while re-base please ensure compilation is successful.
          5. TestNMLeveldbStateStoreService, ln 986 : has compilation error, requires re-base.
          6. TestNMLeveldbStateStoreService, ln 939 : Need to handle positive scenario too when the app can be recovered successfully
          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Ajith S , Patch does not seem to apply now, please rebase the patch and submit the path for jenkins to kick in. Overall approach seems to be fine, except for these few nits ContainerManagerImpl, ln 377: "Container killed after recovery." , may be better message like "Due to invalid StateStore info container was killed during recovery"" ContainerManagerImpl, ln 386 : new line not req. NMLeveldbStateStoreService, ln 268 : requires logging(preferably warn) that the container <containerId> will be killed because of the unknown key <key> during recovery . NMLeveldbStateStoreService, ln 1231 : seems to be already handled by a other patch, while re-base please ensure compilation is successful. TestNMLeveldbStateStoreService, ln 986 : has compilation error, requires re-base. TestNMLeveldbStateStoreService, ln 939 : Need to handle positive scenario too when the app can be recovered successfully
          Hide
          ajithshetty Ajith S added a comment -

          Thanks Naganarasimha G R for your comments. I have attached the latest patch after handing review comments

          Show
          ajithshetty Ajith S added a comment - Thanks Naganarasimha G R for your comments. I have attached the latest patch after handing review comments
          Hide
          varun_saxena Varun Saxena added a comment -

          Ajith S, I think we can raise another JIRA for adding a new table to the state store that will contain a list of container keys and the processing required i.e. killing or skipping if key is not identified when performing rolling downgrades.

          Show
          varun_saxena Varun Saxena added a comment - Ajith S , I think we can raise another JIRA for adding a new table to the state store that will contain a list of container keys and the processing required i.e. killing or skipping if key is not identified when performing rolling downgrades.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 3s trunk passed
          +1 compile 0m 27s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 27s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 41s trunk passed
          +1 javadoc 0m 18s trunk passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 24s the patch passed
          -1 javac 0m 24s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 1 new + 15 unchanged - 1 fixed = 16 total (was 16)
          -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 7 new + 134 unchanged - 0 fixed = 141 total (was 134)
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 48s the patch passed
          +1 javadoc 0m 15s the patch passed
          +1 unit 14m 57s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          28m 53s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e809691
          JIRA Issue YARN-5547
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12837712/YARN-5547.03.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0ad94b0f066a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / ca33bdd
          Default Java 1.8.0_101
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-YARN-Build/13806/artifact/patchprocess/diff-compile-javac-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13806/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13806/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13806/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 3s trunk passed +1 compile 0m 27s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 41s trunk passed +1 javadoc 0m 18s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 24s the patch passed -1 javac 0m 24s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager generated 1 new + 15 unchanged - 1 fixed = 16 total (was 16) -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 7 new + 134 unchanged - 0 fixed = 141 total (was 134) +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 48s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 14m 57s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 28m 53s Subsystem Report/Notes Docker Image:yetus/hadoop:e809691 JIRA Issue YARN-5547 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12837712/YARN-5547.03.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0ad94b0f066a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / ca33bdd Default Java 1.8.0_101 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-YARN-Build/13806/artifact/patchprocess/diff-compile-javac-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13806/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13806/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13806/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Ajith S for the patch, Seems like overall approach is fine, and patch is fine to be committed. Would wait for a day more before committing so that others can review.
          cc/ Jason Lowe.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Ajith S for the patch, Seems like overall approach is fine, and patch is fine to be committed. Would wait for a day more before committing so that others can review. cc/ Jason Lowe .
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for updating the patch!

          Is there a good reason to store the killed state when we aren't going to recover a container? It seems unnecessary to me. If for some reason we crash during recover and try to recover again on the next startup, it will continue to not recognize the container and try to kill it again. Explicitly storing the state as killed doesn't seem to accomplish much. Is there a recovery scenario where it's needed?

          When the container does finally get killed and is removed from the state store, we will leak any keys that are not known by the current software. The state store container removal code only deletes the keys it knows about. We either need to track unknown keys associated with containers or do a scan to remove all keys when we delete a container (the latter could be expensive in terms of latency). If we do go with the latter, we only need to do so for any containers that were recovered, and it would be nice to avoid the performance penalty for containers that don't need it.

          Show
          jlowe Jason Lowe added a comment - Thanks for updating the patch! Is there a good reason to store the killed state when we aren't going to recover a container? It seems unnecessary to me. If for some reason we crash during recover and try to recover again on the next startup, it will continue to not recognize the container and try to kill it again. Explicitly storing the state as killed doesn't seem to accomplish much. Is there a recovery scenario where it's needed? When the container does finally get killed and is removed from the state store, we will leak any keys that are not known by the current software. The state store container removal code only deletes the keys it knows about. We either need to track unknown keys associated with containers or do a scan to remove all keys when we delete a container (the latter could be expensive in terms of latency). If we do go with the latter, we only need to do so for any containers that were recovered, and it would be nice to avoid the performance penalty for containers that don't need it.
          Hide
          vvasudev Varun Vasudev added a comment -

          Ajith S - any updates on this?

          Show
          vvasudev Varun Vasudev added a comment - Ajith S - any updates on this?
          Hide
          ajithshetty Ajith S added a comment -

          Hi guys, sorry for delay. Jason Lowe thanks for your comments. You are right, we can avoid storing killed state for container which will not be recovered. Also, for deleting the unknown keys, would it be ok to remove unknown keys in NMLeveldbStateStoreService.loadContainerState(ContainerId, LeveldbIterator, String) .? As per the patch it will be after the warning log about the unknown keys
          This will avoid any scanning of store hence forth avoid performance penalty

          Show
          ajithshetty Ajith S added a comment - Hi guys, sorry for delay. Jason Lowe thanks for your comments. You are right, we can avoid storing killed state for container which will not be recovered. Also, for deleting the unknown keys, would it be ok to remove unknown keys in NMLeveldbStateStoreService.loadContainerState(ContainerId, LeveldbIterator, String) .? As per the patch it will be after the warning log about the unknown keys This will avoid any scanning of store hence forth avoid performance penalty
          Hide
          jlowe Jason Lowe added a comment -

          for deleting the unknown keys, would it be ok to remove unknown keys in NMLeveldbStateStoreService.loadContainerState(ContainerId, LeveldbIterator, String) .?

          That should be OK as long as we record the container as killed before we remove the unknown keys. When we eventually add the ability to ignore unknown keys without killing the container then it can be problematic. For example:

          1. NM is on version V and is using key K, which is new in version V, that is not deemed critical to the recovery of a running container.
          2. NM is downgraded to version V-1
          3. On startup, NM with version V-1 deletes the unknown key K for the container but keeps it running because it was deemed safe to ignore in the (yet to be added) state store key descriptor table
          4. With the container still running, NM is upgraded to version V again
          5. Now the container has lost key K yet was started on NM version V and continues to run on NM version V.

          If we skip the unknown keys that are deemed "safe to ignore" then we can leak per the concern above if the container completes on version V-1. One way to fix that case is to have the NM always try to delete the list of unknown keys in the (yet to be added) safe-to-ignore key descriptor table when the container completes. Should be fine unless that table gets to be particularly large. But we don't have to implement that now, only when we add the ability to ignore unknown keys without killing a container. For the purposes of this JIRA, we will always be killing containers that have unknown keys so it's simpler.

          Show
          jlowe Jason Lowe added a comment - for deleting the unknown keys, would it be ok to remove unknown keys in NMLeveldbStateStoreService.loadContainerState(ContainerId, LeveldbIterator, String) .? That should be OK as long as we record the container as killed before we remove the unknown keys. When we eventually add the ability to ignore unknown keys without killing the container then it can be problematic. For example: NM is on version V and is using key K, which is new in version V, that is not deemed critical to the recovery of a running container. NM is downgraded to version V-1 On startup, NM with version V-1 deletes the unknown key K for the container but keeps it running because it was deemed safe to ignore in the (yet to be added) state store key descriptor table With the container still running, NM is upgraded to version V again Now the container has lost key K yet was started on NM version V and continues to run on NM version V. If we skip the unknown keys that are deemed "safe to ignore" then we can leak per the concern above if the container completes on version V-1. One way to fix that case is to have the NM always try to delete the list of unknown keys in the (yet to be added) safe-to-ignore key descriptor table when the container completes. Should be fine unless that table gets to be particularly large. But we don't have to implement that now, only when we add the ability to ignore unknown keys without killing a container. For the purposes of this JIRA, we will always be killing containers that have unknown keys so it's simpler.
          Hide
          ajithshetty Ajith S added a comment -

          Thanks for the detail explanation Jason Lowe
          I have updated the patch with expected approach. Please review

          Show
          ajithshetty Ajith S added a comment - Thanks for the detail explanation Jason Lowe I have updated the patch with expected approach. Please review
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 14m 27s trunk passed
          +1 compile 0m 32s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 28s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 50s trunk passed
          +1 javadoc 0m 18s trunk passed
          +1 mvninstall 0m 27s the patch passed
          +1 compile 0m 27s the patch passed
          +1 javac 0m 27s the patch passed
          -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 9 new + 130 unchanged - 0 fixed = 139 total (was 130)
          +1 mvnsite 0m 29s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 54s the patch passed
          +1 javadoc 0m 16s the patch passed
          +1 unit 13m 26s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          35m 32s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5547
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12848481/YARN-5547.04.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 2087b346a728 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / e015b56
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14721/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14721/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/14721/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 14m 27s trunk passed +1 compile 0m 32s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 50s trunk passed +1 javadoc 0m 18s trunk passed +1 mvninstall 0m 27s the patch passed +1 compile 0m 27s the patch passed +1 javac 0m 27s the patch passed -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 9 new + 130 unchanged - 0 fixed = 139 total (was 130) +1 mvnsite 0m 29s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 54s the patch passed +1 javadoc 0m 16s the patch passed +1 unit 13m 26s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 35m 32s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5547 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12848481/YARN-5547.04.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2087b346a728 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e015b56 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14721/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14721/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/14721/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for updating the patch!

          We're still storing a redundant killed state upon recovery.

          'unknownKeysuffix' s/b 'unknownKeySuffix'

          'unknownKeysForConatainer' s/b 'unknownKeysForContainer'

          Show
          jlowe Jason Lowe added a comment - Thanks for updating the patch! We're still storing a redundant killed state upon recovery. 'unknownKeysuffix' s/b 'unknownKeySuffix' 'unknownKeysForConatainer' s/b 'unknownKeysForContainer'
          Hide
          ajithshetty Ajith S added a comment -

          Jason Lowe thanks for the comments
          I have updated the patch

          {without storing killed state + checkstyle issues}
          Show
          ajithshetty Ajith S added a comment - Jason Lowe thanks for the comments I have updated the patch {without storing killed state + checkstyle issues}
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 12m 23s trunk passed
          +1 compile 0m 31s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 25s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 41s trunk passed
          +1 javadoc 0m 17s trunk passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 24s the patch passed
          +1 javac 0m 24s the patch passed
          -0 checkstyle 0m 15s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 2 new + 130 unchanged - 0 fixed = 132 total (was 130)
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 45s the patch passed
          +1 javadoc 0m 14s the patch passed
          +1 unit 13m 1s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          32m 10s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-5547
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12848895/YARN-5547.05.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 531ac4bd167d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 3fa0d54
          Default Java 1.8.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14733/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14733/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/14733/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 12m 23s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 41s trunk passed +1 javadoc 0m 17s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 24s the patch passed +1 javac 0m 24s the patch passed -0 checkstyle 0m 15s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 2 new + 130 unchanged - 0 fixed = 132 total (was 130) +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 45s the patch passed +1 javadoc 0m 14s the patch passed +1 unit 13m 1s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 32m 10s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-5547 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12848895/YARN-5547.05.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 531ac4bd167d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 3fa0d54 Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14733/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14733/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/14733/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          +1 for the latest patch. It doesn't apply cleanly to branch-2. Could you post a patch for that as well?

          Show
          jlowe Jason Lowe added a comment - +1 for the latest patch. It doesn't apply cleanly to branch-2. Could you post a patch for that as well?
          Hide
          ajithshetty Ajith S added a comment -

          attaching patch for branch-2

          Show
          ajithshetty Ajith S added a comment - attaching patch for branch-2
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 15m 39s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 41s branch-2 passed
          +1 compile 0m 28s branch-2 passed with JDK v1.8.0_121
          +1 compile 0m 30s branch-2 passed with JDK v1.7.0_121
          +1 checkstyle 0m 21s branch-2 passed
          +1 mvnsite 0m 32s branch-2 passed
          +1 mvneclipse 0m 15s branch-2 passed
          +1 findbugs 1m 0s branch-2 passed
          +1 javadoc 0m 19s branch-2 passed with JDK v1.8.0_121
          +1 javadoc 0m 20s branch-2 passed with JDK v1.7.0_121
          +1 mvninstall 0m 26s the patch passed
          +1 compile 0m 25s the patch passed with JDK v1.8.0_121
          +1 javac 0m 25s the patch passed
          +1 compile 0m 29s the patch passed with JDK v1.7.0_121
          +1 javac 0m 29s the patch passed
          -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 2 new + 132 unchanged - 0 fixed = 134 total (was 132)
          +1 mvnsite 0m 29s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 7s the patch passed
          +1 javadoc 0m 15s the patch passed with JDK v1.8.0_121
          +1 javadoc 0m 17s the patch passed with JDK v1.7.0_121
          +1 unit 13m 42s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_121.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          60m 9s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue YARN-5547
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12849112/YARN-5547.05.branch-2.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 89d5e8665947 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 26c4cfb
          Default Java 1.7.0_121
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14741/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt
          JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14741/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/14741/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 15m 39s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 41s branch-2 passed +1 compile 0m 28s branch-2 passed with JDK v1.8.0_121 +1 compile 0m 30s branch-2 passed with JDK v1.7.0_121 +1 checkstyle 0m 21s branch-2 passed +1 mvnsite 0m 32s branch-2 passed +1 mvneclipse 0m 15s branch-2 passed +1 findbugs 1m 0s branch-2 passed +1 javadoc 0m 19s branch-2 passed with JDK v1.8.0_121 +1 javadoc 0m 20s branch-2 passed with JDK v1.7.0_121 +1 mvninstall 0m 26s the patch passed +1 compile 0m 25s the patch passed with JDK v1.8.0_121 +1 javac 0m 25s the patch passed +1 compile 0m 29s the patch passed with JDK v1.7.0_121 +1 javac 0m 29s the patch passed -0 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 2 new + 132 unchanged - 0 fixed = 134 total (was 132) +1 mvnsite 0m 29s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 7s the patch passed +1 javadoc 0m 15s the patch passed with JDK v1.8.0_121 +1 javadoc 0m 17s the patch passed with JDK v1.7.0_121 +1 unit 13m 42s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_121. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 60m 9s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue YARN-5547 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12849112/YARN-5547.05.branch-2.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 89d5e8665947 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 26c4cfb Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14741/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14741/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/14741/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          +1 for the branch-2 patch as well. Committing this.

          Show
          jlowe Jason Lowe added a comment - +1 for the branch-2 patch as well. Committing this.
          Hide
          jlowe Jason Lowe added a comment -

          Thanks, Ajith S! I committed this to trunk and branch-2.

          Show
          jlowe Jason Lowe added a comment - Thanks, Ajith S ! I committed this to trunk and branch-2.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11166 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11166/)
          YARN-5547. NMLeveldbStateStore should be more tolerant of unknown keys. (jlowe: rev a33ce45e35ce77dbf297df618aec3106eafda68c)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/recovery/NMLeveldbStateStoreService.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/recovery/TestNMLeveldbStateStoreService.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/recovery/NMStateStoreService.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11166 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11166/ ) YARN-5547 . NMLeveldbStateStore should be more tolerant of unknown keys. (jlowe: rev a33ce45e35ce77dbf297df618aec3106eafda68c) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/recovery/NMLeveldbStateStoreService.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/recovery/TestNMLeveldbStateStoreService.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/recovery/NMStateStoreService.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java

            People

            • Assignee:
              ajithshetty Ajith S
              Reporter:
              jlowe Jason Lowe
            • Votes:
              0 Vote for this issue
              Watchers:
              17 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development