search

apache / kvrocks-controller

Apache Kvrocks Controller is a cluster management tool for Apache Kvrocks.
https://kvrocks.apache.org/
Apache License 2.0
78 stars 42 forks source link

Avoid creating too many Redis clients by reusing #187

Closed erwadba closed 1 week ago

erwadba commented 1 week ago

fix #186 Using map to store redis session

erwadba commented 1 week ago

cc @git-hulk

git-hulk commented 1 week ago

@erwadba Thanks for your contribution.

git-hulk commented 1 week ago

@erwadba Would you mind fixing the lint error?

erwadba commented 1 week ago

@erwadba Would you mind fixing the lint error?

done.

git-hulk commented 1 week ago

@erwadba No worry about this error, it’s not related to this PR. I will merge this PR after fixing it.

erwadba commented 1 week ago

https://github.com/apache/kvrocks-controller/blob/3a336a9b88faaef3678f2cfcdf81f67ae23d9d14/store/cluster.go#L48-L54 https://github.com/apache/kvrocks-controller/blob/3a336a9b88faaef3678f2cfcdf81f67ae23d9d14/controller/controller_test.go#L35-L40 Does --replica 1 means one node in kvrocks cluter? It doesn't look like redis --replica 1 means 2 node

git-hulk commented 1 week ago

@erwadba No worry about this error, it’s not related to this PR. I will merge this PR after fixing it.

Would be fixed in PR: https://github.com/apache/kvrocks-controller/pull/188

git-hulk commented 1 week ago

https://github.com/apache/kvrocks-controller/blob/3a336a9b88faaef3678f2cfcdf81f67ae23d9d14/store/cluster.go#L48-L54

https://github.com/apache/kvrocks-controller/blob/3a336a9b88faaef3678f2cfcdf81f67ae23d9d14/controller/controller_test.go#L35-L40

Does --replica 1 means one node in kvrocks cluter? It doesn't look like redis --replica 1 means 2 node

Yes, --replica 1 means only one replica(master only).

codecov-commenter commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 43.28%. Comparing base (f4f3be0) to head (85c958c).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## unstable #187 +/- ## ============================================ - Coverage 43.35% 43.28% -0.08% ============================================ Files 37 37 Lines 2973 2969 -4 ============================================ - Hits 1289 1285 -4 Misses 1544 1544 Partials 140 140 ``` | [Flag](https://app.codecov.io/gh/apache/kvrocks-controller/pull/187/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/apache/kvrocks-controller/pull/187/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.28% <100.00%> (-0.08%)` | :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=apache#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.