[Documentation] Retention policy documentation is misleading and not accurate

[lhotari]

Describe the bug

Currently it's easy to misunderstand how retention policy should be configured. Both size and time should be set.
It's easy to do mistakes in the configuration. If one sets time to some value and sets size to 0, this will lead to the situation where there is no retention.

https://pulsar.apache.org/docs/en/2.6.1/cookbooks-retention-expiry/#set-retention-policy doesn't explicitly document the meaning of 0 and -1. There's an example, but no explicit documentation like there is in the Javadoc at

pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java

Lines 415 to 417 in 22b8923

	* A retention size of 0, will make data to be deleted immediately.
	* <p>
	* A retention size of -1, means to have an unlimited retention size.

In the documentation there is a sentence "Retention policies are either a size limit or a time limit." . This seems to be wrong since the limit is always based on both size and time. There's also a sentence "It is also possible to set unlimited retention time or size by setting -1 for either time or size retention." (these sentences are in the cookbook's "Retention Policies" section https://pulsar.apache.org/docs/en/2.6.1/cookbooks-retention-expiry/#retention-policies).
This sentence is also misleading since it says "by setting -1 for either time or size retention".
For completely unlimited retention, both values have to be set to -1.

Expected behavior

Retention policy is documented in a clear and accurate way.

Additional context

• https://apache-pulsar.slack.com/archives/C5Z4T36F7/p1603326655482900

Related issue

• Documentation for "Set retention policy" has wrong parameters #655

[lhotari]

I now checked the Javadoc for ManagedLedgerConfig.setRetentionTime .

"A retention time of 0 (the default), will to have no time based retention.
Specifying a negative retention time will make the data to be retained indefinitely, based on the
setRetentionSizeInMB value."

pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java

Lines 386 to 388 in 22b8923

	* A retention time of 0 (the default), will to have no time based retention.
	* <p>
	* Specifying a negative retention time will make the data to be retained indefinitely, based on the

This is surprising since the value 0 seems to have a different meaning for retention time compared to the meaning of value 0 for retention size.

"A retention size of 0, will make data to be deleted immediately.
A retention size of -1, means to have an unlimited retention size."

pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java

Lines 415 to 417 in 22b8923

	* A retention size of 0, will make data to be deleted immediately.
	* <p>
	* A retention size of -1, means to have an unlimited retention size.

The Javadoc for ManagedLedgerConfig.setRetentionTime is wrong.

This can be verified in code:

pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java

Lines 1994 to 2002 in 5df23b5

	private boolean hasLedgerRetentionExpired(long ledgerTimestamp) {
	if (config.getRetentionTimeMillis() < 0) {
	// Negative retention time equates to infinite retention
	return false;
	}
	long elapsedMs = clock.millis() - ledgerTimestamp;
	return elapsedMs > config.getRetentionTimeMillis();
	}

In this code, retention time of 0 will make data to be deleted immediately.

The javadoc specifies the meaning of zero value as "no time based retention". This would have no practical difference to how the javadoc describes the meaning of a negative value: "Specifying a negative retention time will make the data to be retained indefinitely, based on the setRetentionSizeInMB value". This is also "no time based retention".
The javadoc for ManagedLedgerConfig.setRetentionTime should also be fixed. Something like this would be explicit:
"A retention time of 0, will make data to be deleted immediately.
A retention time of -1, means to have an unlimited retention time. In this case, the overall retention condition will be based on retention size."