prometheusremotewrite: release lock when wal empty #29297
Conversation
Previously, the wal tailing routine held an exclusive lock over the entire wal, while waiting for new sections to be written. This led to a deadlock situtation, as a locked wal naturally can not be written to. To remedy this, the lock is now only held during actual read attempts, not in between. Furthermore, inotify watching of the wal dir has been replaced with channel-based signaling between the producers and the tailing routine, so that the latter blocks until any new writes have happened.
Initializes the WAL with a default nop logger, instead of previous nil which led to panics.
Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
| func (prwe *prweWAL) try(index uint64) (*prompb.WriteRequest, error) { | ||
| prwe.mu.Lock() | ||
| defer prwe.mu.Unlock() | ||
| prwe.log.Debug("read", zap.Uint64("index", index)) | ||
| protoBlob, err := prwe.wal.Read(index) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var req prompb.WriteRequest | ||
| if err := proto.Unmarshal(protoBlob, &req); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| prwe.rWALIndex.Add(1) | ||
| return &req, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
why move this up? isn't this highly specific functionality internal to the readFromWAL method?
| } | ||
|
|
||
| return &wal, nil | ||
| log: logger, |
There was a problem hiding this comment.
the beginning of the run() method replaces this logger with one from the context – may lead to unexpected inconsistencies
probably best to remove it from run
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description:
Link to tracking Issue:
Closes #19363
Testing:
Documentation:
cc @sh0rez @MovieStoreGuy @Aneurysm9