tsuna / gohbase

Pure-Go HBase client
Apache License 2.0
737 stars 214 forks source link

SendBatch: Don't start goroutines #232

Closed aaronbee closed 1 year ago

aaronbee commented 1 year ago

Instead of starting goroutines we can queue each region server's collection of RPCs and only after queueing them all wait for all responses. We get a similar amount of parallelism, the only potential slow down is that we can't queue one region server's RPCs until the previous is able to be queued.

By not starting goroutines, the resource usage of SendBatch is more predictable. And not starting goroutines is likely to be less expensive overall.

This change also adds a new metric for number of splits that occur in SendBatch.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01% :warning:

Comparison is base (701886e) 70.30% compared to head (a67f669) 70.30%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #232 +/- ## ========================================== - Coverage 70.30% 70.30% -0.01% ========================================== Files 27 27 Lines 3718 3721 +3 ========================================== + Hits 2614 2616 +2 + Misses 989 988 -1 - Partials 115 117 +2 ``` | [Files Changed](https://app.codecov.io/gh/tsuna/gohbase/pull/232?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Benoit+Sigoure) | Coverage Δ | | |---|---|---| | [rpc.go](https://app.codecov.io/gh/tsuna/gohbase/pull/232?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Benoit+Sigoure#diff-cnBjLmdv) | `85.07% <100.00%> (+0.40%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/tsuna/gohbase/pull/232/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Benoit+Sigoure)

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

aaronbee commented 1 year ago

Testing shows this didn't affect performance, so I think this is worth merging because it simplifies thinking about the cost of a SendBatch.