Add waitForProcessComplete

[zklgame]

Why make this pull request?

implement WaitForProcessComplete:

• use channel to wait for process execution completion
• the WaitForProcessCompletionChannelsPerShardImpl is added at shard-level
• use waitingRequestCreatedAt to record how many times to singal to the same channel, and when to remove expired waiting requests

[longquanzheng]

The lock needs to be a shared instance across threads in order to work as expected

Thanks, Quanzheng

On Tue, May 21, 2024 at 10:48 PM Kaili Zhu @.***> wrote:

@.**** commented on this pull request.

In engine/immediate_task_concurrent_processor.go https://github.com/xcherryio/xcherry/pull/126#discussion_r1609294757:

@@ -66,17 +67,48 @@ func (w immediateTaskConcurrentProcessor) GetTasksToProcessChan() chan<- data_m func (w immediateTaskConcurrentProcessor) AddImmediateTaskQueue( shardId int32, tasksToCommitChan chan<- data_models.ImmediateTask, ) (alreadyExisted bool) {

• exists := w.currentShards[shardId]
• w.currentShards[shardId] = true
• w.taskToCommitChans[shardId] = tasksToCommitChan
• lock := sync.RWMutex{}

But what's the issue with the current implementation? It seems to me create a lock when needed is also doable. I am wondering if this is mainly about performance?

— Reply to this email directly, view it on GitHub https://github.com/xcherryio/xcherry/pull/126#discussion_r1609294757, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCQPMY5D5EA53VDLMPNWVDZDQWSLAVCNFSM6AAAAABGAOST5WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANZQGE2DONRVGA . You are receiving this because your review was requested.Message ID: @.***>

[zklgame]

The lock needs to be a shared instance across threads in order to work as expected Thanks, Quanzheng … On Tue, May 21, 2024 at 10:48 PM Kaili Zhu @.> wrote: @*.*** commented on this pull request. ------------------------------ In engine/immediate_task_concurrent_processor.go <#126 (comment)>: > @@ -66,17 +67,48 @@ func (w immediateTaskConcurrentProcessor) GetTasksToProcessChan() chan<- data_m func (w immediateTaskConcurrentProcessor) AddImmediateTaskQueue( shardId int32, tasksToCommitChan chan<- data_models.ImmediateTask, ) (alreadyExisted bool) { - exists := w.currentShards[shardId] - w.currentShards[shardId] = true - w.taskToCommitChans[shardId] = tasksToCommitChan + lock := sync.RWMutex{} But what's the issue with the current implementation? It seems to me create a lock when needed is also doable. I am wondering if this is mainly about performance? — Reply to this email directly, view it on GitHub <#126 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCQPMY5D5EA53VDLMPNWVDZDQWSLAVCNFSM6AAAAABGAOST5WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANZQGE2DONRVGA . You are receiving this because your review was requested.Message ID: @.>

Makes sense. Modified.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 20.89136% with 284 lines in your changes are missing coverage. Please review.

Project coverage is 57.44%. Comparing base (a33e9ff) to head (e61f54a).

Files	Patch %	Lines
engine/wait_for_process_completion_channel.go	17.30%	86 Missing :warning:
service/api/service_impl.go	0.00%	74 Missing :warning:
service/async/service_impl.go	24.17%	68 Missing and 1 partial :warning:
service/async/gin_handler.go	0.00%	25 Missing :warning:
service/api/gin_handler.go	0.00%	21 Missing :warning:
engine/immediate_task_concurrent_processor.go	78.04%	8 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #126 +/- ## ========================================== - Coverage 59.51% 57.44% -2.07% ========================================== Files 92 93 +1 Lines 7279 7609 +330 ========================================== + Hits 4332 4371 +39 - Misses 2666 2953 +287 - Partials 281 285 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.