tsdb: stop saving a copy of last 4 samples in memSeries #11296
Conversation
|
Sadly the unit tests say that the race is not avoided. I will look some more. |
|
I don't think my change to My change assumes the caller never tries to read the last byte if it's unsafe to do. So, in |
bboreham
force-pushed
the
remove-4-sample-copy
branch
2 times, most recently
from
c67f1ab
to
4b030a6
Compare
|
I have updated this PR to take a copy of the last byte in |
Because the data is stored as a bit-stream, the last byte in the stream could change if the stream is appended to after an Iterator is obtained. Copy the last byte when the Iterator is created, so we don't have to read it later. Clarify in comments that concurrent Iterator and Appender are allowed, but the chunk must not be modified while an Iterator is created. (This was already the case, in order to copy the bstream slice header.) Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
This extra copy of the last 4 samples was introduced to avoid a race condition between reading the last byte of the chunk and writing to it. But now we have fixed that by having `bstreamReader` copy the last byte, we don't need to copy the last 4 samples. This change saves 56 bytes per series, which is very worthwhile when you have millions or tens of millions of series. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Previous changes have left this code duplicating some lines; pull them out to a separate function and tidy up. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
bboreham
force-pushed
the
remove-4-sample-copy
branch
from
c698b83
to
c222cd4
Compare
The behaviour has changed so chunk iterators are only wrapped when transaction isolation requires them to stop short of the end. This makes tests fail which are checking the type. Tests should check the observable behaviour, not the type. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
bboreham
force-pushed
the
remove-4-sample-copy
branch
from
c222cd4
to
b844d29
Compare
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
This extra copy of the last 4 samples was introduced to avoid a race condition between reading the last byte of the chunk and writing to it.
However this race condition was fixed in f42ed03, so we don't need the copy any more.EDIT: But we can copy just the last byte in
bstreamReaderinstead.This change saves 56 bytes per series, which is very worthwhile when you have millions of series.
(#11280 and #11288 each remove another 8 bytes)
I kept the head snapshot binary compatible by writing zeros where the samples used to be and ignoring any values found, but perhaps we should bump the version and save the work.