-
Notifications
You must be signed in to change notification settings - Fork 693
Initial search functions for the live_store to mirror what ingester does. #5494
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
Changes from all commits
f1757ae
6c88c96
27332d8
53f33cf
b2240a3
f898500
73a8b8e
8b0d99d
ca9c7ee
cdd022f
58124ad
4f50f1a
18d1623
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ package livestore | |
|
|
||
| import ( | ||
| "context" | ||
| "flag" | ||
| "sync" | ||
| "time" | ||
|
|
||
|
|
@@ -29,7 +30,7 @@ type instance struct { | |
| enc encoding.VersionedEncoding | ||
|
|
||
| // Block management | ||
| blocksMtx sync.Mutex | ||
| blocksMtx sync.RWMutex | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed more fine grained mutex when dealing with blocks.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rwmutex's are slower than plain ol' mutexes. unless it's heavy on reads a lot of times a plain mutex is faster. no idea what the ratio is.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ingester code used RWMutex so assuming we thought about that then :) |
||
| headBlock common.WALBlock | ||
| walBlocks map[uuid.UUID]common.WALBlock | ||
| completeBlocks map[uuid.UUID]*ingester.LocalBlock | ||
|
|
@@ -72,7 +73,7 @@ func newInstance(instanceID string, wal *wal.WAL, overrides Overrides, logger lo | |
| } | ||
|
|
||
| func (i *instance) pushBytes(ts time.Time, req *tempopb.PushBytesRequest) { | ||
| if len(req.Traces) >= len(req.Ids) { | ||
| if len(req.Traces) != len(req.Ids) { | ||
| level.Error(i.logger).Log("msg", "mismatched traces and ids length", "IDs", len(req.Ids), "traces", len(req.Traces)) | ||
| return | ||
| } | ||
|
|
@@ -228,6 +229,13 @@ func (i *instance) cutBlocks(immediate bool) (uuid.UUID, error) { | |
| return id, nil | ||
| } | ||
|
|
||
| var blockConfig = common.BlockConfig{} | ||
|
|
||
| func init() { | ||
| // TODO MRD this is a hack until we roll the config into the livestore | ||
| blockConfig.RegisterFlagsAndApplyDefaults("", &flag.FlagSet{}) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we dont do this bloom filters try to apply int64 max transformations.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. odd we're passing in a defaulted block config. should we not add that to the config struct in config.go? |
||
| } | ||
|
|
||
| func (i *instance) completeBlock(ctx context.Context, id uuid.UUID) error { | ||
| i.blocksMtx.Lock() | ||
| walBlock := i.walBlocks[id] | ||
|
|
@@ -249,7 +257,7 @@ func (i *instance) completeBlock(ctx context.Context, id uuid.UUID) error { | |
| } | ||
| defer iter.Close() | ||
|
|
||
| newMeta, err := i.enc.CreateBlock(ctx, &common.BlockConfig{}, walBlock.BlockMeta(), iter, reader, writer) | ||
| newMeta, err := i.enc.CreateBlock(ctx, &blockConfig, walBlock.BlockMeta(), iter, reader, writer) | ||
| if err != nil { | ||
| level.Error(i.logger).Log("msg", "failed to create complete block", "id", id, "err", err) | ||
| return err | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with the fork. hopefully we will delete this soon.