Skip to content

Conversation

@aadhikar
Copy link
Contributor

@aadhikar aadhikar commented Feb 3, 2026

Description: The postentry check in PR #7077 was broken - postentry is always NULL at that point, fixed by removing the check.

Relates: #7076

Reviewed by: @vashirov

Summary by Sourcery

Bug Fixes:

  • Fix revert_cache() not being called during parent modrdn operations when BETXN callbacks fail due to an incorrect postentry check.

Description: The postentry check in PR 389ds#7077 was broken - postentry is always NULL
at that point, fixed by removing the check.

Relates: 389ds#7076

Reviewed by: @vashirov
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Consider expanding the new comment above the if (parent_op && betxn_callback_fails) block to briefly capture the previous nuance about only reverting when BETXN POST plugin callbacks fail (i.e., when cache modifications may have occurred), so future readers understand why betxn_callback_fails is the right guard.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider expanding the new comment above the `if (parent_op && betxn_callback_fails)` block to briefly capture the previous nuance about only reverting when BETXN POST plugin callbacks fail (i.e., when cache modifications may have occurred), so future readers understand why `betxn_callback_fails` is the right guard.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

*/
if (parent_op && betxn_callback_fails && postentry) {
/* Revert the caches if this is the parent operation */
if (parent_op && betxn_callback_fails) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change makes me nervous because the comments specifically state that you only need to revert the cache if you hit the postop phase (where postentry is not NULL). Did you verify if a postop plugin fails that postentry is still NULL?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants