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.

Sign up for GitHub

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

Jump to bottom

Introduces per stream chunks mutex #3000

Merged
merged 63 commits into from
Nov 27, 2020
Merged

Introduces per stream chunks mutex #3000

merged 63 commits into from
Nov 27, 2020

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Nov 27, 2020 •
edited
Loading

Followup to #2981

Introduces a chunks mutex for synchronization of access to stream chunks across appends, transfers, flushes, reads, and checkpointing.

Although there have been slight modifications due to PR feedback on 2981, this variant has been running for ~3 days without issue.

@owen-d
marshalable chunks
1b2b931
@owen-d
wal record types custom serialization
ddecf02
@owen-d
proto types for wal checkpoints
48fa77f
@owen-d
byteswith output unaffected by buffer
7ec0fb0
@owen-d
wal & record pool ifcs
d6ccccf
@owen-d
wal record can hold entries from multiple series
eb6a519
@owen-d
entry pool
f089f09
@owen-d
ingester uses noopWal
211356d
@owen-d
removes duplicate argument passing in ingester code. adds ingester co…
7adf966 
…nfig validation & derives chunk encoding.
@owen-d
Merge remote-tracking branch 'upstream/master' into wal/writes
29fafd6
@owen-d
segment writing
5ed8ffb
@owen-d
[WIP] wal recovery from segments
107b03b
@owen-d
replay uses sync.Maps & preserves WAL fingerprints
fb194f5
@owen-d
in memory wal recovery
3d36b2d
@owen-d
wal segment recovery
9a7eaa5
@owen-d
ingester metrics struct
30cdbf2
@owen-d
wal replay locks streamsMtx in instances, adds checkpoint codec
d723c0b
@owen-d
ingester metrics
897196e
@owen-d
checkpointer
8df2598
@owen-d
WAL checkpoint writer
b5c2ef0
@owen-d
checkpointwriter can write multiple checkpoints
60ebe5e
@owen-d
reorgs checkpointing
ed14e0c
@owen-d
wires up checkpointwriter to wal
5acf745
@owen-d
ingester SeriesIter impl
6dd28b1
@owen-d
wires up ingesterRecoverer to consume checkpoints
a7e40fd
@owen-d
generic recovery fn
957717b
@owen-d
generic recovery fn
9384e98
@owen-d
recover from both wal types
a1beb69
@owen-d
cleans up old tmp checkpoints & allows aborting in flight checkpoints
4e27f7c
@owen-d
wires up wal checkpointing
45840bc
@owen-d
Merge remote-tracking branch 'upstream/master' into wal/writes
d4318c7
@owen-d
segment replay ignores out of orders
d43ecde
@owen-d
fixes bug between WAL reading []byte validity and proto unmarshalling…
ce72f33 
… refs
@owen-d
conf validations, removes comments
c9614aa
@owen-d
flush on shutdown config
571f8fc
@owen-d
POST /ingester/shutdown
2c965f3
@owen-d
renames flush on shutdown
1c4d526
@owen-d
wal & checkpoint use same segment size
d9139de
@owen-d
Merge remote-tracking branch 'upstream/master' into wal/writes
9a4c810
@owen-d
writes entries to wal regardless of tailers
bd9cee0
@owen-d
makes wal checkpoing duration default to 5m
e042812
@owen-d
recovery metrics
f632022
@owen-d
encodes headchunks separately for wal purposes
a948785
@owen-d
merge upstream
2e972c7
@owen-d
typo
43d8a78
@owen-d
linting
3166183
@owen-d
adds chunkMtx to ingester streams
9474697
@owen-d
all flush locking locks streams -> chunks in order to prevent deadlocks
9e97f48
@owen-d
Merge remote-tracking branch 'upstream/master' into wal/chunks-mtx
6a35f91
@owen-d
wal integration tests
697ee85
@owen-d
addresses pr feedback
ee38a1f 
uses entry pool in stream push/tailer

removes unnecessary pool interaction

checkpointbytes comment

fillchunk helper, record resetting in tests via pool

redundant comment

defers wg done in recovery

s/num/count/

checkpoint wal uses a logger

encodeWithTypeHeader now creates its own []byte

removes pool from decodeEntries

wal stop can error
@owen-d
Merge remote-tracking branch 'upstream/master' into wal/chunks-mtx
a60252b
@owen-d
cleans up merge
844e0ba
cyriltovena
cyriltovena approved these changes Nov 27, 2020
View reviewed changes
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM.

May be a defer would be better, but you need another function for this, you can't defer in a for loop.

@owen-d
Copy link
Member Author

owen-d commented Nov 27, 2020

Yeah I was also thinking about a sync.Once for releasing 🤔

@owen-d owen-d merged commit 8c1fe88 into grafana:master Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyriltovena cyriltovena cyriltovena approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@owen-d @cyriltovena