Add comment explainign potential risks about duplicates prevention

This commit is contained in:
Konstantin Knizhnik
2023-05-15 17:25:00 +03:00
parent 6837356cc1
commit 51256890a0
+16
View File
@@ -3389,6 +3389,22 @@ impl Timeline {
// NOTE: this is racy, if there can be any other task that concurrently
// creates L1 layers. Currently, there can be only one compaction task
// running at any time, so this is fine.
//
// Also we hold `layer_removal_cs` guard which should prevent race condition
// even if there are two or more concurrent compaction tasks.
//
// But there is an opposite issue: we check presence of duplicates under
// `layers` shared lock, but then it is released. So there is a gap between
// this check and adding new layer to layer map. In principle in this gap some
// some other task (i.e. GC) can drop this layer and we already abandon insertion
// of duplicate layer. As a result there will be no such layer at all.
// In other words: we have some state S1 of pageserver where layer L1 can be removed by GC.
// Then we run compaction and it switch pageserver to the state S2 which writes duplicate of
// layer L1 and where it can not be removed. With this patch it is possible that
// we switch pageserver to state S2 but... with L1 lost.
// It is just hypothetical situation and there is no such concrete scenario which
// reproduces this problem. So let's take this risk.
//
if self.layers.read().unwrap().contains(
&(w.key_start()..end_key),
&w.lsn_range(),