embed: value of AutoCompactionRetention and embed.CompactorModeRevision are incompatible

[grepory]

Specifying embed.CompactorModeRevision for the configuration's AutoCompactionMode parameter will result in an unintentional value for the revision compactor's retention parameter.

Because of the treatment of retention as a time.Duration and its conversion here -- we end up passing a 1 hour time.Duration and converting it to an int64 via int64(retention), which tells the revision compactor to only compact after revision 3600000000000.

To get around this, we're specifying the retention value for CompactorModeRevision in nanoseconds which parses as a "1ns" -> 1, for example.

[grepory]

Also, I'd assume this isn't just affecting embedded etcd, but have not loooked into how the command line does it for etcd.

[gyuho]

@grepory How do you configure embed.Config?

We define AutoCompactionRetention as time.Duration in order to support both compact modes (revision-based, interval-based).

Because of the treatment of retention as a time.Duration

If you need periodic compaction, you can:

// auto-compaction every hour
cfg.AutoCompactionMode = embed.CompactorModePeriodic
cfg.AutoCompactionRetention = "1h"

If you need revision-based compaction, you can:

// auto-compaction every 5000 rev
cfg.AutoCompactionMode = embed.CompactorModeRevision
cfg.AutoCompactionRetention = "5000"

I don't think you need to convert time.Hour? Just passing "1h" should work?

[grepory]

AutoCompactionRetention gets cased to a time.Duration no matter what mode you specify.

At some point, that value gets multiplied by time.Hour -- at which point 5000 becomes 5000 hours which gets converted to an int64 -- which is much more than 5000 revisions. Right now, we're doing this:

	// Every 5 minutes, we will prune all values in etcd to only their latest
	// revision.
	cfg.AutoCompactionMode = embed.CompactorModeRevision
	// This has to stay in ns until https://github.com/coreos/etcd/issues/9337
	// is resolved.
	cfg.AutoCompactionRetention = "1ns"

Specifying only "1" in AutoCompactionRetention ended up specifying a revision count of 3600000000000. Again, where this happens is here:

https://github.com/coreos/etcd/blob/6f76e46c0cdbc1d9721dcbc290ce2e41e3577586/embed/etcd.go#L147

1 hour does not translate to 1 revision.

[gyuho]

Specifying only "1" in AutoCompactionRetention ended up specifying a revision count of 3600000000000.

Oh now I see the issue.

time.Duration(int64(h)) * time.Hour was there only for backward compatibility, but we did it wrong.

Thanks for the report.

We will fix this soon!

/cc @fanminshi

[grepory]

No problem! Thanks for looking into it. :) <3

[gyuho]

@grepory Should be fixed by #9339. Will backport to 3.3 as well. Thanks.