Retention Policy Inconsistencies

[corylanou]

There are inconsistencies in how retention policies are created and
altered in our system. I will attempt to outline the current behavior and then
we can come to a consensus on the desired behavior.

Create retention policy without default and with default

If you have a retention policy that is not the default, and then create it
again, with the same specifications, but add DEFAULT when creating it, it
will actually make that the default RP, even though it was already created.

> CREATE DATABASE metrics
> SHOW RETENTION POLICIES ON metrics
name    duration        shardGroupDuration      replicaN        default
----    --------        ------------------      --------        -------
autogen 0s              168h0m0s                1               true

> CREATE RETENTION POLICY daily ON metrics DURATION 1d REPLICATION 1
> SHOW RETENTION POLICIES ON metrics
name    duration        shardGroupDuration      replicaN        default
----    --------        ------------------      --------        -------
autogen 0s              168h0m0s                1               true
daily   24h0m0s         1h0m0s                  1               false

> CREATE RETENTION POLICY daily ON metrics DURATION 1D REPLICATION 1 DEFAULT
> show retention policies on metrics
name    duration        shardGroupDuration      replicaN        default
----    --------        ------------------      --------        -------
autogen 0s              168h0m0s                1               false
daily   24h0m0s         1h0m0s                  1               true

Since the replication policy is already created, creating it again with a
different specification (in this case DEFAULT) should error out with
retention policy conflicts with an existing policy.

This is a bug currently, but fixing it will result in a breaking change. I'm
not sure what our stance is on breaking changes when it fixes a bug that was
undesired behavior.

Clarifying when DEFAULT retention policies should be created.

When using the CREATE DATABASE statement and including retention policy
information, that retention policy should get created, and become the default.
No other retention policy (not even autogen if it's enabled, should be
created).

Example when specifying retention policy.

> CREATE DATABASE "foo" with NAME "thing"
> SHOW RETENTION POLICIES ON foo
name    duration        shardGroupDuration      replicaN        default
----    --------        ------------------      --------        -------
thing   0s              168h0m0s                1               true

Example when not specifying retention policy.

> CREATE DATABASE "foo"
> SHOW RETENTION POLICIES ON foo
name    duration        shardGroupDuration      replicaN        default
----    --------        ------------------      --------        -------
autogen 0s              168h0m0s                1               true

This is the current behavior. I am just confirming that this is still correct.

[e-dard]

This is a bug currently, but fixing it will result in a breaking change. I'm
not sure what our stance is on breaking changes when it fixes a bug that was
undesired behavior.

In my opinion this is not a breaking change, it's a bug fix. If DROP DATABASE didn't actually remove a database in some cases, I wouldn't consider fixing that a breaking change. I don't see how this is any different. It seems to me like the current behaviour is not desirable and that the second invocation of the CREATE RETENTION POLICY ... DEFAULT should return an error.