Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

Meta tag support bigtable #1646

Merged
merged 14 commits into from
Mar 3, 2020
Merged

Meta tag support bigtable #1646

merged 14 commits into from
Mar 3, 2020

Conversation

replay
Copy link
Contributor

@replay replay commented Jan 29, 2020

This implements storing meta records in a new bigtable meta record index. Additionally it refactors the cassandra index so that the meta record index is now also split from the rest of the cassandra index in it's own pkg. It moves some of the config parameters for the cassandra backed meta record index around, so if this code change gets accepted then i'll update the breaking changes log accordingly.

@Dieterbe
Copy link
Contributor

It moves some of the config parameters for the cassandra backed meta record index around,

this seems weird. especially because so many settings that are normally shared (hosts, keyspace, auth, timeout, ...) are now duplicated. Why would anyone want to store half of their index data in one cassandra cluster and the other half in a different cluster (or keyspace)? This seems very unlikely and unnecessarily complicated.
It also seems annoying to update settings in 2 places and having to keep them in sync. and can lead to trouble if a mistake is made in this process.

@replay
Copy link
Contributor Author

replay commented Jan 29, 2020

The idea is to split the whole meta tag index away from the current main index and into it's pkg. I could just copy the config settings of the main index into the meta tag index as well, but then why did we split the store and the index into separate config sections? Isn't the argument there to just provide as much flexibility as possible?

@replay replay force-pushed the meta_tag_support_bigtable branch 3 times, most recently from 01c4ee3 to 1009fbc Compare January 30, 2020 00:00
Copy link
Contributor

@robert-milan robert-milan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few things to start

api/graphite.go Show resolved Hide resolved
idx/cassandra/cassandra.go Outdated Show resolved Hide resolved
idx/metatags/meta_record_status.go Show resolved Hide resolved
@replay replay force-pushed the meta_tag_support_bigtable branch 3 times, most recently from e873d79 to f1361c2 Compare February 17, 2020 22:17
this type gets moved into the type idx because we'll also want to use it
from the bigtable meta records index.
@replay replay force-pushed the meta_tag_support_bigtable branch 2 times, most recently from 41db82a to 674979f Compare March 3, 2020 00:50
@replay replay force-pushed the meta_tag_support_bigtable branch from b128cc3 to 674979f Compare March 3, 2020 01:57
@replay
Copy link
Contributor Author

replay commented Mar 3, 2020

I rebased this PR and went over all the changes one more time to make sure everything is ok, and to familiarize myself with it again.

I didn't make any large changes, @robert-milan you should be able to see the diff of the changes I made today by doing git diff 41db82 e3df92 after fetching.

Out of all the changes I made, the largest one was in store/bigtable/bigtable.go because there I'm now using the EnsureTableExists() utility from bigtable/util.go, which previously wasn't used there.

If you're still ok with these recent changes, then I'll go ahead and merge.

@robert-milan
Copy link
Contributor

I rebased this PR and went over all the changes one more time to make sure everything is ok, and to familiarize myself with it again.

I didn't make any large changes, @robert-milan you should be able to see the diff of the changes I made today by doing git diff 41db82 e3df92 after fetching.

Out of all the changes I made, the largest one was in store/bigtable/bigtable.go because there I'm now using the EnsureTableExists() utility from bigtable/util.go, which previously wasn't used there.

If you're still ok with these recent changes, then I'll go ahead and merge.

Sure, go ahead

@replay replay merged commit 82dedf9 into master Mar 3, 2020
@replay replay deleted the meta_tag_support_bigtable branch March 3, 2020 14:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants