Skip to content

Conversation

@luohoufu
Copy link
Contributor

What does this PR do

Rationale for this change

Standards checklist

  • The PR title is descriptive
  • The commit messages are semantic
  • Necessary tests are added
  • Updated the release notes
  • Necessary documents have been added if this is a new feature
  • Performance tests checked, no obvious performance degradation

@luohoufu luohoufu requested a review from Copilot November 1, 2025 04:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an offset management issue in the bulk indexing worker by ensuring the offset is set to ctx1.NextOffset before submitting bulk requests, rather than using the obsolete pop.NextOffset value from individual messages.

Key Changes

  • Updated offset assignment timing to use ctx1.NextOffset before bulk request submission in two locations
  • Removed the obsolete assignment of offset = &pop.NextOffset after successful submission

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}
committedOffset = offset
} else {
log.Error("offset not committed:", offset, ",moved to:", &pop.NextOffset)
Copy link

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

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

The variable pop is undefined at this location. This line is inside the successful submission block (line 950-964), but the loop variable pop from line 831 is not accessible here. This will cause a compilation error. Consider either removing this log line or replacing &pop.NextOffset with &ctx1.NextOffset.

Suggested change
log.Error("offset not committed:", offset, ",moved to:", &pop.NextOffset)
log.Error("offset not committed:", offset, ",moved to:", &ctx1.NextOffset)

Copilot uses AI. Check for mistakes.
@luohoufu luohoufu closed this Nov 3, 2025
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.

3 participants