Skip to content

Fix[mqb]: drain on closeChannel#1221

Merged
dorjesinpo merged 1 commit into
mainfrom
fix/drain-channel
Apr 6, 2026
Merged

Fix[mqb]: drain on closeChannel#1221
dorjesinpo merged 1 commit into
mainfrom
fix/drain-channel

Conversation

@dorjesinpo

Copy link
Copy Markdown
Collaborator

mqbnet::Channel::closeChannel should close channel (asynchronously) only after sending all items unless channel is closed

@dorjesinpo dorjesinpo requested a review from a team as a code owner March 26, 2026 02:11
@dorjesinpo dorjesinpo added the bug Something isn't working label Mar 26, 2026
@dorjesinpo dorjesinpo changed the title drain on closeChannel Fix[mqb]: drain on closeChannel Mar 26, 2026
@dorjesinpo dorjesinpo requested a review from 678098 March 26, 2026 11:29
@dorjesinpo dorjesinpo assigned hallfox and unassigned 678098 Mar 26, 2026
@dorjesinpo dorjesinpo requested review from hallfox and removed request for 678098 March 26, 2026 15:25

d_state.testAndSwap(e_READY, e_CLOSE);
}
wakeUp();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How are we guaranteed that we'll be in the IDLE mode for the channel close logic to kick in here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The mode can be any after d_isClosing.store(true);
If it is e_BLOCK, the wakeUp(); should wake it up and the mode goes the cycle
e_BLOCK -> e_FLUSH_BUFFER -> e_IDLE -> e_BLOCK

This logic is based on the fact that after wakeUp();, the mode must arrive at e_IDLE where d_isClosing gets picked up (unless resetChannel()/setChannel() happens which means the channel is closed).

hallfox
hallfox previously approved these changes Mar 31, 2026

@hallfox hallfox left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

Comment thread src/groups/mqb/mqbnet/mqbnet_channel.h Outdated
Comment on lines 353 to 360

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We can remove explicit values or start them from 0

// PUBLIC TYPES
enum EnumState {
/// Not connected
e_INITIAL = 0,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This enum value is still mentioned in the component's docs.
These docs are also outdated:

// Internally, 'mqbnet::Channel' starts a new thread and unconditionally
// buffers everything it needs to write into single consumer queue. The thread
// reads the channel state which can be
// - e_INITIAL, not connected
// - e_RESET, indicating a need to reset because of a connection change;
// - e_READY, low-watermark
// - e_HWM, high-watermark
// If the state is 'e_RESET', the thread clear the queue and transitions to
// 'e_INITIAL' . If the channel is set and the state is e_INITIAL, it
// transitions to 'e_READY'. It the state is not e_READY, the thread waits on
// condition variable until the state is e_READY. If the state is 'e_READY',
// the thread pop an Item from the queue and attempts to write.
// Three public methods can change the state:
// - 'resetChannel' sets the state to 'e_RESET';
// - 'setChannel' sets the state to 'e_RESET' and waits for the internal
// thread to transition to 'e_READY'. This is done to
// make sure, any write succeeds after 'setChannel'
// returns. The transition 'e_RESET' -> 'e_READY' is
// signaled by conditional variable.
// - 'onWatermark' sets the state to 'e_READY' on LWM and 'e_HWM' on HWM.

Signed-off-by: dorjesinpo <129227380+dorjesinpo@users.noreply.github.com>
@dorjesinpo

Copy link
Copy Markdown
Collaborator Author

(unrelated) IT failure is analyzed and the fix is created #1257

@dorjesinpo dorjesinpo merged commit 636a473 into main Apr 6, 2026
76 of 87 checks passed
@dorjesinpo dorjesinpo deleted the fix/drain-channel branch April 6, 2026 16:00
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