Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: blockbuilder component #14621

Merged
merged 61 commits into from
Nov 22, 2024
Merged

feat: blockbuilder component #14621

merged 61 commits into from
Nov 22, 2024

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Oct 25, 2024

Introduces a "slimgester" component, which is essentially an ingester without write ahead logs, querying support (inc inverted indices or in-mem TSDB implementation), or limits application. It consumes a "job", aka(partition, offset_range) in kafka, writing our pre-existing format (chunks) to object storage. When finished, a TSDB is created from these chunks and written to storage.

This creates a way to idempotently consume a job specification and commit it in a more optimized form to a storage backend. We'll use this to iterate later on new storage formats, a scheduler+worker architecture for building, etc.

See plan.txt in this PR for more detail.

owen-d added 15 commits October 21, 2024 10:26
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
Signed-off-by: Owen Diehl <[email protected]>
@owen-d owen-d changed the title RF1 Ingestion feat: blockbuilder component Nov 14, 2024
@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Nov 14, 2024
pkg/blockbuilder/controller.go Outdated Show resolved Hide resolved
"err", err,
)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

log and continue? this would fail the service


for _, db := range built {
u := newUploader(i.objStore)
if err := u.Put(ctx, db); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: retries to avoid processing the job again

}

select {
case ch <- converted:
Copy link
Contributor

Choose a reason for hiding this comment

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

was this intentional to send all the inputs in one go? this won't make use of the parallel workers from the next stage.

pkg/blockbuilder/slimgester.go Outdated Show resolved Hide resolved
@owen-d owen-d marked this pull request as ready for review November 22, 2024 05:58
@owen-d owen-d requested a review from a team as a code owner November 22, 2024 05:58
Copy link
Contributor

@ashwanthgoli ashwanthgoli left a comment

Choose a reason for hiding this comment

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

LGTM!

return periodicConfigs[i].From.Time.Before(periodicConfigs[j].From.Time)
})
for _, periodicConfig := range periodicConfigs {
objectClient, err := storage.NewObjectClient(periodicConfig.ObjectType, "storage-rf1", storageConfig, clientMetrics)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also wrap this with NewPrefixedObjectClient if PathPrefix is set

defer m.mtx.RUnlock()

// TODO(owen-d): safe to remove?
// Remove __name__="logs" as it's not needed in TSDB
Copy link
Contributor

Choose a reason for hiding this comment

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

should be safe to remove as we are not adding this in the first place when creating the chunk?

@ashwanthgoli ashwanthgoli merged commit cbdd36a into grafana:main Nov 22, 2024
59 of 60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants