codecov-commenter
commented
1 week ago Codecov Report
Attention: Patch coverage is 90.67797% with 11 lines in your changes missing coverage. Please review.
Project coverage is 41.33%. Comparing base (
08c3ba6) to head (5ca728f). Report is 11 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| internal/pool/pool.go | 93.81% | 6 Missing :warning: |
| internal/query/result_set.go | 66.66% | 5 Missing :warning: |
Additional details and impacted files
```diff @@ Coverage Diff @@ ## master #1307 +/- ## ========================================== - Coverage 41.35% 41.33% -0.02% ========================================== Files 321 321 Lines 33782 33816 +34 ========================================== + Hits 13969 13978 +9 - Misses 19344 19364 +20 - Partials 469 474 +5 ``` | [Flag](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | Coverage Δ | | |---|---|---| | [go-1.20.x](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | `43.56% <90.67%> (-0.02%)` | :arrow_down: | | [go-1.21.x](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | `43.56% <90.67%> (-0.03%)` | :arrow_down: | | [go-1.22.x](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | `41.30% <90.67%> (-0.03%)` | :arrow_down: | | [macOS](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | `40.88% <90.19%> (+0.02%)` | :arrow_up: | | [ubuntu](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | `40.86% <90.19%> (-0.02%)` | :arrow_down: | | [unit](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | `41.33% <90.67%> (-0.02%)` | :arrow_down: | | [windows](https://app.codecov.io/gh/ydb-platform/ydb-go-sdk/pull/1307/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform) | `41.27% <90.67%> (-0.07%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ydb-platform#carryforward-flags-in-the-pull-request-comment) to find out more.:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Current approach has couple shortcomings:
What is the new behavior?
I propose to use buffered channel to manage query pool items as oppose to index map and idle slice. This approach makes item creation process continuous and asynchronous, which allows seamlessly warm-up pool or re-establish all connections after connectivity failure.
Internally I introduced:
getItem() and putItem() are working with item queue - receiving or sending back items. New thing about getItem() is that it ensures that retrieved item is ok to use (as far IsAlive() can tell). To achieve this it spins in endless loop until in can receive item from queue. This means in case of connectivity failure or pool starvation it will block causing back pressure to caller (who should use context with deadline if it wishes to not block as well but detect this situation).
Other information
To discuss:
In this approach 'index' stat is not used. Should we get rid of it if the changes will be given green light?