morningman commented on a change in pull request #8728:
URL: https://github.com/apache/incubator-doris/pull/8728#discussion_r837332798
##########
File path: be/src/exec/tablet_sink.cpp
##########
@@ -458,8 +458,37 @@ int NodeChannel::try_send_and_fetch_status(RuntimeState*
state,
return 0;
}
bool is_finished = true;
- if (!_add_batch_closure->is_packet_in_flight() && _pending_batches_num > 0
&&
- _last_patch_processed_finished.compare_exchange_strong(is_finished,
false)) {
+
+ // Synchronization here is complicated actually, every one should pay much
attention.
+ // Both packet_in_flight and _last_patch_processed_finished are of
std::atomic and
+ // are used to guarantee only one task is submitted to thread_pool_token.
+ // Direct accessing on std::atomic uses the memory order of
std::memory_order_seq_cst.
+ // https://en.cppreference.com/w/cpp/atomic/atomic/operator_T
+ // https://en.cppreference.com/w/cpp/atomic/atomic/load
+ // https://en.cppreference.com/w/cpp/atomic/atomic/operator%3D
+ // https://en.cppreference.com/w/cpp/atomic/atomic/store
+ // https://en.cppreference.com/w/cpp/atomic/memory_order
+ // Here we should cmp_exchange _last_patch_processed_finished before
reading
+ // packet_in_flight because try_send_patch sets packet_in_flight before
+ // _last_patch_processed_finished.
+ // If we read _add_batch_closure before cmp_exchange
_last_patch_processed_finished,
+ // then somting bad would happend. e.g.
+ //
|----------------------------------------------------------------------------
+ // | Thread try_send_and_fetch_status | Thread try_send_batch
|
+ //
|-----------------------------------------------------------------------------
+ // | read packet_in_flight false | before seting packet_in_flight
|
Review comment:
```suggestion
// | read packet_in_flight false | before setting packet_in_flight
|
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
