This was introduced by YARN-5221. Sample stacktrace:

2016-09-06 17:24:19,258 [main] INFO service.AbstractService: Service org.apache.hadoop.yarn.server.nodemanager.containermanager.ContainerManagerImpl failed in state INITED; cause: java.io.IOException: Unexpected container state key: ContainerManager/containers/container_e44_1472715025911_0001_01_000002/version
java.io.IOException: Unexpected container state key: ContainerManager/containers/container_e44_1472715025911_0001_01_000002/version
        at org.apache.hadoop.yarn.server.nodemanager.recovery.NMLeveldbStateStoreService.loadContainerState(NMLeveldbStateStoreService.java:243)
        at org.apache.hadoop.yarn.server.nodemanager.recovery.NMLeveldbStateStoreService.loadContainersState(NMLeveldbStateStoreService.java:182)
        at org.apache.hadoop.yarn.server.nodemanager.containermanager.ContainerManagerImpl.recover(ContainerManagerImpl.java:267)
        at org.apache.hadoop.yarn.server.nodemanager.containermanager.ContainerManagerImpl.serviceInit(ContainerManagerImpl.java:251)
        at org.apache.hadoop.service.AbstractService.init(AbstractService.java:163)
        at org.apache.hadoop.service.CompositeService.serviceInit(CompositeService.java:107)
        at org.apache.hadoop.yarn.server.nodemanager.NodeManager.serviceInit(NodeManager.java:263)
        at org.apache.hadoop.service.AbstractService.init(AbstractService.java:163)
        at org.apache.hadoop.yarn.server.nodemanager.NodeManager.initAndStartNodeManager(NodeManager.java:507)
        at org.apache.hadoop.yarn.server.nodemanager.NodeManager.main(NodeManager.java:555)

Ideally we should not store a version key unless we are using the feature that requires it. In other words, if the container version is 0 then we don't store the key and instead infer that if the version key is missing then the container version must be zero. I assume we already do this when upgrading from 2.7 to 2.8. This would preserve the ability to downgrade as long as nothing uses the increase container resource feature that needs the new version key. If something uses the increase container resource capability only then do we lose the ability to downgrade.

Patch that only stores the container version key if the version is non-zero.

-1 overall

This message was automatically generated.

The TestQueuingContainerManager failure is not related and being tracked by YARN-5377.

Updating the patch to address the checkstyle issue. It's now twice as large, so yay checkstyle.

+1 overall

This message was automatically generated.

Like you mentioned in YARN-5633, maybe we should update the schema version only if the new feature is used (only if storeContainerResourceChange method in the stateStore is called). This means we should move the storeVersion method from the checkVersion to inside the storeContainerResourceChange method.

Yeah I'm not sure what to do about the schema version. Ideally I'd like to not bump it unless absolutely necessary. Since we lack any kind of downgrade script it makes it a permanent, one-way trip for the upgrade. It's already incompatible today if these keys appear in the state store because it doesn't recognize them. If later a container runs that did use an increment we're incompatible while that container is running, but when it completes and the state is removed from the store then we're compatible again. If we upgrade the schema version then we're not, even after the containers in question are gone. Thoughts?

Good point...
One option is to

1. keep an in memory reference count of : new_NM_store_key - > running containers are using new NM store keys (we can track this by incrementing if storeContainerResourceChange is called or storeContainer is called with version > 0 and decrementing when removeContainer is called for containers whose version > 0)
2. ensure NEW Db version is written only during NM shutdown. If any new_NM_store_key has a count > 0, increment dbVersion.

We could do the refcounting thing, but what I'm thinking is that we don't need to mess with the schema version key at all. The presence of a container version key already acts like a new schema version since it causes the old NM to not survive startup. That's exactly what incrementing the schema version would do, so there's not a lot of net benefit by going through the complicated refcount incr/decr schema thing. Either a container version key is present and prevents the old NM from starting up or no container version key exists and the old NM can startup like before.

Is there a compelling reason to bother updating the schema version given the presence of a container version key (or really any unrecognized container key) will prevent the old software from starting?

Is there a compelling reason to bother updating the schema version given the presence of a container version key (or really any unrecognized container key) will prevent the old software from starting?

Hmmm... given that the IOExecption in the loadContainer will bomb anyway.. guess it should be fine then

I feel this is a much needed capability as upgrades are never successful all the time. Thanks Jason Lowe for working on and Arun Suresh for the thoughtful reviews. The approach is definitely a good improvement on the current state, I think we should address the problem holistically along with YARN-5547. I have given a suggestion there.

Subru Krishnan, I like your suggestion of getting of marking keys as safe to read or not... and, yes we should externalize this list in a table (or a file).
But we still have to deal with 2.8 -> 2.7 rollback. Are you suggesting we introduce an intermediate version that has just the fix for the StateStore (externalized file / table of keys) move to that version.. and then from there perform the actual upgrade to 2.8 ?

But we still have to deal with 2.8 -> 2.7 rollback. Are you suggesting we introduce an intermediate version that has just the fix for the StateStore (externalized file / table of keys) move to that version.. and then from there perform the actual upgrade to 2.8?

Yes as I feel only then is 2.8 -> 2.7 rollback addressed. For practical reasons, I am fine if we do them separately but feel we should track them together (in a single rollback umbrella jira).

Agreed. Another option to maybe consider is have an external script (or maybe even start the NodeManager using a special --prepare-for-rollback arg) which takes a list of allowed keys, iterates through all the entries in the LevenDb store and scrub entries with keys not in the provided list.
This also allows us to make the fix in our target (2.8) version.

I'm not a fan of the "prepare for rollback" approach if we can avoid it. It adds another user-visible phase to the rollback procedure and places the burden on admins, requiring them to either know what keys are valid/appropriate to specify for the command or that they need to run a special script which embeds this knowledge. Also simply removing the keys from the database is not going to be a proper downgrade procedure. Those keys represent state that is important to preserve on a restart, and if we ignore it then we are dropping a user request for a container. That's not going to be OK in the general case, as that may prevent a container from launching properly or having the proper properties when it is launched. Depending upon the nature of the feature that added the new store keys, we may not be able to support the downgrade at all short of failing the container because we can't execute it as requested.

In the short term I think we should commit something similar to this patch to unblock the 2.8 release. IMHO we should be OK if we support downgrades from 2.8 to 2.7 if the user does not leverage the new features in 2.8 (i.e.: container increase/decrease, queuing, etc.). Once those features are used then a downgrade may not work. This mirrors what was done for the epoch number in container IDs between 2.5 and 2.6. Downgrades worked as long as the new work-preserving RM restart wasn't performed after upgrading to 2.6. In general if we are careful only to use new store keys when they are absolutely necessary then we can support rollbacks as long as users don't use the new features added in the new release.

After unblocking 2.8 we can then work on the data-driven key ignoring in YARN-5547. That will help cover another set of features where a simple delete of the keys is sufficient to perform the downgrade. That would then leave the features where we can't just ignore keys, and we'll have to come up with some other approach or state to users that downgrades do not necessarily work once that new feature is being used.

In that case... +1 for the latest patch..

it adds another user-visible phase to the rollback procedure and places the burden on admins, requiring them to either know what keys are valid/appropriate to specify ..

Not necessarily. We can check in a rollback file that contains all new keys included in every compatible CURRENT_VERSION_INFO which can be scrubbed safely before rollback.

....
1.3
/version

1.2
/logDir
/workDir

1.1
/queued
...

rel 2.8 corresponds to ver 1.3 version info. and rel 2.7 corresponds to 1.0. The --prepare-for-rollback flag just takes a target version_info argument (in this case is 1.0). And now we have a list of keys to scrub without any deep involvement from the admin (Additionally, we can choose to to Key the file with the release version rather than the CURRENT_VERSION_INFO to make it even easier)

Thanks for the +1, Arun! I'll commit this later today if there are no objections.

Regarding the key removal, yes if we read the keys out of the store itself then that totally makes sense. Actually I was assuming that during the rollback when the old software saw a key that was ignored it would just remove it as it iterated the database. I suppose we could have the entry flag whether or not the ignored key should be preserved as long as the container is active in case there is a subsequent roll-forward. Then there's no need for a prepare-for-rollback once the support for the key table is in the old software version and all we need to do is delete keys. Of course if there's more to do than just delete keys or fail containers then something needs to be run on the new software before downgrading to the old one.

I committed this to trunk, branch-2, and branch-2.8.

SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10433 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10433/)
YARN-5630. NM fails to start after downgrade from 2.8 to 2.7. (jlowe: rev e7933097354a246b080b46f1a4ca2ef0f39f3b38)

• (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/NMLeveldbStateStoreService.java