diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 9838640097..65aad07257 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -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(),