ZOOKEEPER-4915: Default znode.container.maxNeverUsedIntervalMs to 5 minutes

[kezhuw]

This will delete container nodes that never had any children after approximately 5 minutes.

Given this property, you should be prepared to get KeeperException.NoNodeException
when creating children inside of this container node

Container nodes are supposed to be deleted after all children deleted, so from client's perspective this change has no harm. And also, it leaves no unused nodes in data tree.

[zeekling]

The default 5-minute cleanup is a breaken change and is not recommended i think.

[kezhuw]

How does it break things ? Container nodes with no children are supposed to be deleted.

zookeeper/zookeeper-server/src/main/java/org/apache/zookeeper/CreateMode.java

Lines 49 to 58 in 9d1d25c

	/**
	* The znode will be a container node. Container
	* nodes are special purpose nodes useful for recipes such as leader, lock,
	* etc. When the last child of a container is deleted, the container becomes
	* a candidate to be deleted by the server at some point in the future.
	* Given this property, you should be prepared to get
	* {@link org.apache.zookeeper.KeeperException.NoNodeException}
	* when creating children inside of this container node.
	*/
	CONTAINER(4, false, false, true, false),

[anmolnar]

Change looks good to me.
Any unit tests to add?

[kezhuw]

@anmolnar Thank you for reviewing! Test added in fixup commit.

[anmolnar]

@anmolnar Thank you for reviewing! Test added in fixup commit.

Test only validates the default configuration. Why don't you make a test which creates a container node, waits and validates it has been deleted.

[kezhuw]

Why don't you make a test which creates a container node, waits and validates it has been deleted.

CreateContainerTest::testMaxNeverUsedInterval has already covered this.

[anmolnar]

Why don't you make a test which creates a container node, waits and validates it has been deleted.

CreateContainerTest::testMaxNeverUsedInterval has already covered this.

Now I see it. Previously the default value was indefinite, but you change it to 5 minutes in this patch. Okay, let's do this.

[anmolnar]

Approved.