TSDB shipper + WAL #6049
TSDB shipper + WAL #6049
Conversation
pkg/loki/modules.go
Outdated
| t.Cfg.StorageConfig.BoltDBShipperConfig.IngesterDBRetainPeriod = shipperQuerierIndexUpdateDelay(t.Cfg.StorageConfig.IndexCacheValidity, t.Cfg.StorageConfig.BoltDBShipperConfig.ResyncInterval) + 2*time.Minute | ||
|
|
||
| t.Cfg.StorageConfig.TSDBShipperConfig.Mode = indexshipper.ModeWriteOnly | ||
| t.Cfg.StorageConfig.TSDBShipperConfig.IngesterName = t.Cfg.Ingester.LifecyclerConfig.ID |
There was a problem hiding this comment.
The reason ingester name is not being set with target all is because t.Cfg.isModuleEnabled would only look at explicitly specified targets in -target flag. We will have to move it to a separate block with or without the check using t.Cfg.isModuleActive("ingester") check which actually checks whether the module is active or not .
write fingerprints without synthetic tenant label strip out tenant labels from queries
There was a problem hiding this comment.
One thing I noticed is that the new shipper code is not adding ingester names to the files, having just the timestamp. This can cause multiple ingesters uploading files at exactly the same time to override each other's files. I will take care of this issue in a follow-up PR that I will work on to make the boltdb-shipper client use the new shipper code.
| } | ||
|
|
||
| func (w *ChunkWriter) PutOne(ctx context.Context, from, through model.Time, chk chunk.Chunk) error { | ||
| log, ctx := spanlogger.New(ctx, "SeriesStore.PutOne") |
There was a problem hiding this comment.
| log, ctx := spanlogger.New(ctx, "SeriesStore.PutOne") | |
| log, ctx := spanlogger.New(ctx, "TSDBStore.PutOne") |
| } | ||
|
|
||
| func (m *HeadManager) Append(userID string, ls labels.Labels, chks index.ChunkMetas) error { | ||
| labelsBuilder := labels.NewBuilder(ls) |
There was a problem hiding this comment.
might be an overkill to use a label builder for removing a single label. It create 2 arrays of 5. Or should we reuse it via a pool ? I think just removing the labels and sorting does the job.
| now := time.Now() | ||
| curPeriod := m.period.PeriodFor(now) | ||
|
|
||
| toRemove, err := m.shippedTSDBsBeforePeriod(curPeriod) |
There was a problem hiding this comment.
Can you add a comment on this, not I fully understand how you know they are shipped.
| for _, id := range wals { | ||
| // use anonymous function for ease of cleanup | ||
| if err := func(id WALIdentifier) error { | ||
| reader, closer, err := wal.NewWalReader(walPath(dir, id.ts), -1) |
There was a problem hiding this comment.
Why not a io.ReaderCloser ?
There was a problem hiding this comment.
The reader is a *wal.Reader, not an io.Reader
| return err | ||
| } | ||
|
|
||
| if len(rec.Series.Labels) > 0 { |
There was a problem hiding this comment.
I guess the expectations here is that the labels will always appear in the wal before the chunks. I think it's worth a small coment.
| if !ok { | ||
| return nil, nil | ||
| } | ||
| defer unlock() |
There was a problem hiding this comment.
Why not handling RW lock inside the head code ? I get that we need RW lock to add/get new heads in the map, but seems like adding another RW per TSDB would be better because it will scale with the amount of tenant and not be bounded to the shard.
This PR does a few things and is very large. I'm opening as a draft to get some initial feedback. It:
(TSDB Head, TSDB WAL, indexshipper)This is by no means complete, but it's getting rather large and this is a relatively good place to PR so we can have multiple authors focus on different parts.
On disk, it looks like this when building/shipping the TSDB multitenant indices (first phase):
cc @sandeepsukhani @cyriltovena @slim-bean
ref #5428