receive: Remove MultiTSDBStore and replace with ProxyStore

[bwplotka]

Those two structs are quite complex, but are doing exactly the same thing.

Few (smallish) work Items to make that happen:

• rename storeRef to cachedStoreClient
• create localStoreClient that will allow calling locally some storepb.StoreServer. For receive, it would be TSDBStores. This will need to use our tenantSeriesSetServer or something similar.
• ensure similar spans for tracing and error logging with tenant ids

Motivation

Maintaining two literally same things is tedious, we can simplify things 🤗 Especially that we have nice timeout, chunk dedup etc logics for proxy store we might want to use.

Action Items:

• Thanos Receive is using ProxyStore instead of MultiTSDBStore for fanout Series request to TSDBs\
• MultiTSDB benchmark exists, just is swapped with ProxyStore
• Benchmarks before & after is produced, following script can be used:

#!/usr/bin/env bash

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

cd ${DIR}/../

NUM=${1}
RUN="${NUM}-receiveseries"
mkdir -p ${DIR}/bench_outs/${RUN}

for TEST in BenchmarkMultiTSDBSeries
do
echo "Running ${TEST} run ${RUN}"
go test -bench=${TEST} -run=^$ -benchmem -memprofile ${DIR}/bench_outs/${RUN}/memprofile${TEST//\//-}.out -timeout 2h -benchtime 30s ./pkg/store | tee  ${DIR}/bench_outs/${RUN}/bench${TEST//\//-}.out
done

PREV=$((NUM - 1))
benchstat -delta-test none ${DIR}/bench_outs/${PREV}-receiveseries/BenchmarkMultiTSDBSeries.out ${DIR}/bench_outs/${RUN}/BenchmarkMultiTSDBSeries.out | tee ${DIR}/bench_outs/${RUN}/diff.out

cc @brancz @kakkoyun

[brancz]

There were a few things that I ran into that I felt made this forced and caused odd interface implementations to make use of a "generic" abstraction. If you can come up with something reasonable though I'm all for deduplicating.

[bwplotka]

Yea, I think you might mean Client interface. This is bit more wider than gRPC StoreAPI Server, but still easy to implement something for local TSDBStore to fit (:

Help wanted!

[brancz]

Yeah I still think it needs some refactoring I wouldn't want to press it into an interface that doesn't actually fit, the Addr() function for example makes no sense here. Again, I'm not against it, I just think it needs a larger refactoring, then I think this could work nicely.

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[bwplotka]

Still todo.

…

On Sat, 8 Aug 2020 at 10:42, stale[bot] ***@***.***> wrote: Hello 👋 Looks like there was no activity on this issue for last 30 days. *Do you mind updating us on the status?* Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command <https://probot.github.io/apps/reminders/> if you wish to be reminded at some point in future. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2864 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVA3OYU4QQMAOXRZ2ZUC4TR7UMXNANCNFSM4OU4KEDQ> .

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[bwplotka]

Still valid (: Kind Regards, Bartek Płotka (@bwplotka)

…

On Sat, 7 Nov 2020 at 06:04, stale[bot] ***@***.***> wrote: Hello 👋 Looks like there was no activity on this issue for the last two months. *Do you mind updating us on the status?* Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command <https://probot.github.io/apps/reminders/> if you wish to be reminded at some point in future. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2864 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVA3O3YQFPKA2FEEGJKQTTSOTIPFANCNFSM4OU4KEDQ> .

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[Abhishek357]

still valid

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[yeya24]

Still valid I think.
Something really nice to support this is to have tenant label matching and min/max time filtering at the Series grpc request. This could make the receiver query faster.

[matej-g]

I looked into this TODO as a part of working on something else, I think it should be doable (and yes, some adjustments to the client interface would be nice to make this fit more reasonably). I'll try to come back with a draft PR.

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[matej-g]

Still relevant (still not done due to lack of time 😿)

[stale]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[matej-g]

Resolved via #5552