Skip to content

Conversation

Copy link

Test Results

  7 files  ±0    7 suites  ±0   3m 11s ⏱️ -54s
 54 tests ±0   53 ✅ ±0  1 💤 ±0  0 ❌ ±0 
223 runs  ±0  220 ✅ ±0  3 💤 ±0  0 ❌ ±0 

Results for commit e4cd9db. ± Comparison against base commit f992b69.

Copy link

github-actions bot commented Jul 29, 2025

Test Results

  7 files  ±0    7 suites  ±0   4m 47s ⏱️ -21s
 54 tests ±0   53 ✅ ±0  1 💤 ±0  0 ❌ ±0 
223 runs  ±0  220 ✅ ±0  3 💤 ±0  0 ❌ ±0 

Results for commit 868cebc. ± Comparison against base commit e67c936.

♻️ This comment has been updated with latest results.

@AhmedSoliman AhmedSoliman marked this pull request as ready for review July 29, 2025 17:00
Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

Thanks for simplifying the overall structure by letting the watchdog manage the LogChainExtender. LGTM. +1 for merging :-)

Copy link
Contributor

@pcholakov pcholakov left a comment

Choose a reason for hiding this comment

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

+1 to merge fwiw, this is definitely cleaner! 🐕 Thank you, @AhmedSoliman!

Comment on lines +257 to +259
// ignore in-flight trims if task-center is shutting down.
// we don't want to prematurely shutdown the watchdog as we
// might need it to continue running "during" shutdown.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice touch!

The PR introduces two changes:
- `Bifrost::find_tail()` will only attempt to seal the chain if FindTailOpts asks for consistent result; we don't need to cripple the fast path for find-tail.
- Find tail will eagerly seal the chain without grace period. This is to avoid unnecessary startup delays for single-node setups.

The fact that chain sealing is idempotent makes it lighterweight to perform as it'll coalesce nicely with other reconfigurations that might be happening concurrently.
Allows the system to capture the "source" and the "node" for chain seal markers. There is also a new `--reason` flag to `restatectl log seal` to specify the reason, and the reason + the node id will be printed in `restatectl logs describe <id>` output.
A partial seal is when the tail loglet is sealed but the chain itself is not. The read stream will detect this situation and seal the chain after a grace period (5s+50% jitter).
@AhmedSoliman AhmedSoliman merged commit 1b10ee1 into main Jul 30, 2025
38 checks passed
@AhmedSoliman AhmedSoliman deleted the pr3598 branch July 30, 2025 14:27
@github-actions github-actions bot locked and limited conversation to collaborators Jul 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants