Skip to content

Commit 656ea74

Browse files
authored
[BUG][wal3] GC gets wedged (#4972)
## Description of changes When a log contention error is encountered in GC, it will proceeed to delete without installing the manifest. Further, if an existing GC file is found after this case, the GC will be wedged trying to make garbage from a snapshot that doesn't exist. The fix is to retry applying the manifest until there is no garbage work. And then try loading garbage before generating garbage. ## Test plan CI - [X] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes N/A
1 parent 36a8955 commit 656ea74

2 files changed

Lines changed: 82 additions & 47 deletions

File tree

rust/wal3/src/manifest_manager.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,24 @@ impl ManifestManager {
328328
}
329329
staging.garbage = Some(garbage);
330330
}
331-
self.do_work().await
331+
loop {
332+
{
333+
let staging = self.staging.lock().unwrap();
334+
if staging.garbage.is_none() {
335+
break;
336+
}
337+
}
338+
match self.do_work().await {
339+
Ok(_) => {}
340+
Err(Error::LogContentionRetry)
341+
| Err(Error::LogContentionFailure)
342+
| Err(Error::LogContentionDurable) => {}
343+
Err(err) => {
344+
return Err(err);
345+
}
346+
};
347+
}
348+
Ok(())
332349
}
333350

334351
async fn do_work(&self) -> Result<(), Error> {

rust/wal3/src/writer.rs

Lines changed: 64 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -639,58 +639,76 @@ impl OnceLogWriter {
639639
cutoff
640640
};
641641
self.manifest_manager.heartbeat().await?;
642-
let garbage = self
643-
.manifest_manager
644-
// TODO(rescrv): Evaluate putting a cache in here.
645-
.compute_garbage(options, cutoff, &())
646-
.await?;
647-
let Some(garbage) = garbage else {
648-
return Ok(());
649-
};
650-
let (garbage, e_tag) = match garbage
651-
.install(
652-
&self.options.throttle_manifest,
653-
&self.storage,
654-
&self.prefix,
655-
existing,
656-
)
657-
.await
658-
{
659-
Ok(e_tag) => (garbage, e_tag),
660-
Err(Error::LogContentionFailure)
661-
| Err(Error::LogContentionRetry)
662-
| Err(Error::LogContentionDurable) => {
663-
match Garbage::load(&self.options.throttle_manifest, &self.storage, &self.prefix)
642+
let garbage_and_e_tag =
643+
match Garbage::load(&self.options.throttle_manifest, &self.storage, &self.prefix).await
644+
{
645+
Ok(Some((garbage, e_tag))) => Some((garbage, e_tag)),
646+
Ok(None) => None,
647+
Err(err) => {
648+
return Err(err);
649+
}
650+
};
651+
let (garbage, e_tag) = if let Some((garbage, e_tag)) = garbage_and_e_tag {
652+
(garbage, e_tag)
653+
} else {
654+
let garbage = self
655+
.manifest_manager
656+
// TODO(rescrv): Evaluate putting a cache in here.
657+
.compute_garbage(options, cutoff, &())
658+
.await?;
659+
let Some(garbage) = garbage else {
660+
return Ok(());
661+
};
662+
let (garbage, e_tag) = match garbage
663+
.install(
664+
&self.options.throttle_manifest,
665+
&self.storage,
666+
&self.prefix,
667+
existing,
668+
)
669+
.await
670+
{
671+
Ok(e_tag) => (garbage, e_tag),
672+
Err(Error::LogContentionFailure)
673+
| Err(Error::LogContentionRetry)
674+
| Err(Error::LogContentionDurable) => {
675+
match Garbage::load(
676+
&self.options.throttle_manifest,
677+
&self.storage,
678+
&self.prefix,
679+
)
664680
.await
665-
{
666-
Ok(Some((garbage, e_tag))) => {
667-
if garbage.is_empty() {
668-
if base {
669-
return Err(Error::LogContentionRetry);
681+
{
682+
Ok(Some((garbage, e_tag))) => {
683+
if garbage.is_empty() {
684+
if base {
685+
return Err(Error::LogContentionRetry);
686+
} else {
687+
return Box::pin(self.garbage_collect_recursive(
688+
options,
689+
true,
690+
e_tag.as_ref(),
691+
keep_at_least,
692+
))
693+
.await;
694+
}
670695
} else {
671-
return Box::pin(self.garbage_collect_recursive(
672-
options,
673-
true,
674-
e_tag.as_ref(),
675-
keep_at_least,
676-
))
677-
.await;
696+
(garbage, e_tag)
678697
}
679-
} else {
680-
(garbage, e_tag)
681698
}
682-
}
683-
Ok(None) => {
684-
return Err(Error::LogContentionFailure);
685-
}
686-
Err(err) => {
687-
return Err(err);
699+
Ok(None) => {
700+
return Err(Error::LogContentionFailure);
701+
}
702+
Err(err) => {
703+
return Err(err);
704+
}
688705
}
689706
}
690-
}
691-
Err(err) => {
692-
return Err(err);
693-
}
707+
Err(err) => {
708+
return Err(err);
709+
}
710+
};
711+
(garbage, e_tag)
694712
};
695713
let Some(e_tag) = e_tag else {
696714
return Err(Error::GarbageCollection(

0 commit comments

Comments
 (0)