store: Added test for consistency delay filter, defaulted to 0.

[bwplotka]

Signed-off-by: Bartlomiej Plotka [email protected]

[bwplotka]

cc @khyatisoneji : We missed a unit test for that.

Also moving to default consistency delay to 0. This is to not surprise users. In fact you only need this when you have eventually consistent object storage, am I right? (:

[brancz]

Should we have documentation on what appropriate values might be for various providers? For example with AWS S3's read-after-write consistency we have no problem with a consistency delay of 0 is my understanding.

[bwplotka]

Yes. At least.. we did not notice anything wrong with S3. Eventual consistency can have really different tmp side effects. In theory we kind of DON'T hit the thing mentioned by S3 as the one that triggers eventual consistent behavior: a HEAD against the path that is later used. We only do that for meta.json:

thanos/pkg/shipper/shipper.go

Line 297 in 55cb8ca

ok, err := s.bucket.Exists(ctx, path.Join(m.ULID.String(), block.MetaFilename))

So it's fine I guess. But all of this is not well documented by S3. I think having some small consistency delay might be helpful just in case. Also, this is still crippled as we check block creation time, not the actual modification time of file.

[metalmatze]

As these are tests nothing needs to be added to the Changelog, correct? Just rebase the release branch to include it?!

[bwplotka]

Yup. We already have entry for it.

[brancz]

In theory we kind of DON'T hit the thing mentioned by S3 as the one that triggers eventual consistent behavior: a HEAD against the path that is later used. We only do that for meta.json:

yeah I'm saying writing down expected things and correcting them as we learn also helps the community and users :)

[GiedriusS]

Makes sense to me, this is not to surprise the users. OTOH there's no bad side effect if we will start serving data from remote object storage only after 30 minutes except that it might not be intuitive. All in all, let's opt for the principle of least surprise but some docs would indeed be nice 👍