Skip to content

Fix two bugs around the handling of child nodes of a deleted parent#352

Merged
dumbbell merged 3 commits into
v0.17.xfrom
fix-missing-keep_while-deleted-nodes-in-result
Jan 7, 2026
Merged

Fix two bugs around the handling of child nodes of a deleted parent#352
dumbbell merged 3 commits into
v0.17.xfrom
fix-missing-keep_while-deleted-nodes-in-result

Conversation

@dumbbell
Copy link
Copy Markdown
Collaborator

There were two bugs caused by the lack of handling of child tree nodes implicitly deleted with their parent:

  • Child nodes were missing from the result map.
  • Child nodes deleted because of a parent's keep_while expiring were not fully handled. This led to a leak of child nodes' keep_while conditions in the reverse index.

See each commit for the full explanation.

The third commit introduces test cases to verify both bug fixes.

@dumbbell dumbbell added this to the v0.17.4 milestone Dec 12, 2025
@dumbbell dumbbell requested a review from mkuratczyk December 12, 2025 18:41
@dumbbell dumbbell self-assigned this Dec 12, 2025
@dumbbell dumbbell added the bug Something isn't working label Dec 12, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 12, 2025

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.42%. Comparing base (a99db9d) to head (c499365).
⚠️ Report is 4 commits behind head on v0.17.x.

Files with missing lines Patch % Lines
src/khepri_tree.erl 94.11% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           v0.17.x     #352      +/-   ##
===========================================
- Coverage    86.46%   86.42%   -0.04%     
===========================================
  Files           22       22              
  Lines         3465     3477      +12     
===========================================
+ Hits          2996     3005       +9     
- Misses         469      472       +3     
Flag Coverage Δ
erlang-26 86.10% <95.00%> (-0.01%) ⬇️
erlang-27 86.39% <95.00%> (-0.04%) ⬇️
erlang-28 86.07% <95.00%> (-0.04%) ⬇️
os-ubuntu-latest 86.42% <95.00%> (-0.02%) ⬇️
os-windows-latest 86.10% <95.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dumbbell dumbbell force-pushed the fix-missing-keep_while-deleted-nodes-in-result branch from d2ad562 to 7dd991c Compare January 2, 2026 12:35
Copy link
Copy Markdown
Contributor

@mkuratczyk mkuratczyk left a comment

Choose a reason for hiding this comment

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

thanks!

@dumbbell dumbbell changed the base branch from main to v0.17.x January 5, 2026 13:14
[Why]
We document that the result explicitly lists all deleted tree nodes,
whether directly by a command or indirectly through a `keep_while`
condition to the result map.

This should include children of top-level deleted nodes as well.

[How]
Recursively add all children of a deleted tree node to the result map.
[Why]
This map is used for various tasks. One of them is the cleanup of
`keep_while` conditions and the associated reverse index.

Before this commit, entries were leaked in the reverse index because
only the parent was taken care of.
[Why]
There were two bugs caused by the lack of handling of child tree nodes
implicitly deleted with their parent:
* Child nodes were missing from the result map.
* Child nodes deleted because of a parent's `keep_while` expiring were
  not fully handled. This led to a leak of child nodes' `keep_while`
  conditions in the reverse index.

[How]
The new test cases cover both bugs.

We introduce two new helpers to implement this test:
* `khepri_machine:get_keep_while_conds_revidx/1 to query the
  `keep_while` conditions reverse index.
* `khepri_tree:unopacify/1` to be able to compare opaque reverse indices
  with expected values in test cases.

Each test case correspdons to a version of the machine, and thus a
specific implementation of the reverse index.
@dumbbell dumbbell force-pushed the fix-missing-keep_while-deleted-nodes-in-result branch from 7dd991c to c499365 Compare January 7, 2026 10:22
@dumbbell dumbbell marked this pull request as ready for review January 7, 2026 16:05
@dumbbell dumbbell merged commit a266e2d into v0.17.x Jan 7, 2026
26 checks passed
@dumbbell dumbbell deleted the fix-missing-keep_while-deleted-nodes-in-result branch January 7, 2026 16:05
@michaelklishin
Copy link
Copy Markdown
Contributor

Can/should we produce a new 0.17.x release for 4.2.3?

@dumbbell
Copy link
Copy Markdown
Collaborator Author

dumbbell commented Jan 7, 2026

It will go together with related changes in Horus and RabbitMQ. I don’t remember the date for RabbitMQ 4.2.3, so not sure it will be ready if it is about to be released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants