Remove support for chunks storage entirely #4704
Conversation
aughr
force-pushed
the
no_chunks
branch
8 times, most recently
from
126f31b to
3e61c09
Compare
|
Consider removing chunks option from https://github.com/cortexproject/cortex/blob/master/.github/ISSUE_TEMPLATE/bug_report.md |
Thank you, good catch! |
There was a problem hiding this comment.
This is looking good. I had this build running in dev with all the components (including configs and a ruler). After flags changes I saw everything looking good.
But maybe more things still need to be removed.
In pkg/ingester/ingester.go I see some flushing to dynamodb as well.
CHANGELOG.md
Outdated
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| ## master / unreleased | |||
|
|
|||
| **This release removes support for chunks storage. See below for more.** | |||
|
|
|||
| * [CHANGE] Remove support for chunks storage entirely. If you are using chunks storage on a previous version, you must [migrate your data](https://github.com/cortexproject/cortex/blob/v1.11.1/docs/blocks-storage/migrate-from-chunks-to-blocks.md) on version 1.12 or earlier. Before upgrading to this release, you should also remove any deprecated chunks-related configuration, as this release will no longer accept that. | |||
There was a problem hiding this comment.
while testing this PR I noticed following flags have been removed:
-dynamodb.api-limit
-dynamodb.url
-s3.url
-schema-config-file
-ingester.chunk-encoding
-ingester.max-transfer-retries
I am sure there are more. I feel we should call out specifically these removals in the CHANGELOG. For example:
-dynamodb.* has been removed
| } | ||
|
|
||
| return true | ||
| } |
There was a problem hiding this comment.
pkg/chunk/chunk.go mentions dynamodb a few times. I think more functions can be removed from here, but I am not sure.
There was a problem hiding this comment.
Yeah, this is pretty messy. I'll rip out much of this in an upcoming commit on this PR, but I'll stop before a wholesale refactor to avoid this PR getting even larger.
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
Signed-off-by: Andrew Bloomgarden <[email protected]>
There was a problem hiding this comment.
In docs/architecture.md, the Configs API section mentions PostgreSQL, is that still supported?
|
Closed by: #4812 |
What this PR does: It builds on top of #4679 and removes chunks storage entirely. It also removes the uses of the legacy object store client in the chunks codebase, which requires removing support for the legacy store configs for alertmanager and ruler. Those are marked deprecated today, and we intended to remove them in 1.11.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]