Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SimpleBatcher apparent deadlock #2196 #3148

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ggivo
Copy link
Contributor

@ggivo ggivo commented Jan 31, 2025

Problem:

  • Flush Operation Conflict:
      - When one thread (T1) performs a flush, it may read and dispatch batched commands before resetting the flushing flag.
      - If another thread (T2) adds a command and forced flush at this moment. Command might be added to the queue but does not trigger a flush.
      - As a result, the command remains in the queue until the next flush request, causing a delay in dispatching.

  • Flag Reset Between Iterations:
      - During a default flush operation, if multiple batches are processed, the flushing flag is reset between iterations.
      - This allows another thread to take over, potentially causing the initial thread to return BatchTasks.EMPTY instead of properly processed commands.

  1. T1 -> batch(command, CommandBatching.flush()
  2. T1 -> flushing.compareAndSet(false, true) == true
  3. T1 -> flush()->doFlush()
  4. T2 -> batch(command, CommandBatching.flush()
  5. T2 -> flushing.compareAndSet(false, true) == false  #already flushing will skip doFlush  and command remain not dispatched
  6. T1 -> batch() completes
  7. T2 -> batch() completes

Fix:

If force flush is requested while flushing, perform additional flush iteration after ongoing completes

Make sure that:

  • You have read the contribution guidelines.
  • You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
  • You applied code formatting rules using the mvn formatter:format target. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.

ggivo added 2 commits January 31, 2025 16:31
### Problem:
- **Flush Operation Conflict:**
  - When one thread (T1) performs a flush, it may read and dispatch batched commands before resetting the `flushing` flag.
  - If another thread (T2) adds a command and forced flush at this moment. Command might be added to the queue but does not trigger a flush.
  - As a result, the command remains in the queue until the next flush request, causing a delay in dispatching.

- **Flag Reset Between Iterations:**
  - During a default flush operation, if multiple batches are processed, the `flushing` flag is reset between iterations.
  - This allows another thread to take over, potentially causing the initial thread to return `BatchTasks.EMPTY` instead of properly processed commands.

1. T1 -> batch(command, CommandBatching.flush()
2. T1 -> flushing.compareAndSet(false, true) == true
3. T1 -> flush()->doFlush()
4. T2 -> batch(command, CommandBatching.flush()
5. T2 -> flushing.compareAndSet(false, true) == false  #already flushing will skip doFlush  and command remain not dispatched
6. T1 -> batch() completes
7. T2 -> batch() completes

### Fix: If force flush is requested while flushing, perform additional flush iteration after ongoing completes
@ggivo ggivo self-assigned this Jan 31, 2025
@ggivo ggivo requested a review from tishun January 31, 2025 16:06
@tishun tishun added the type: bug A general bug label Feb 3, 2025
@tishun tishun added this to the 6.6.0.RELEASE milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants