[#73] pool/tree: Support pool for tree service #90
No reviewers
Labels
No Label
P0
P1
P2
P3
good first issue
pool
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-sdk-go#90
Loading…
Reference in New Issue
There is no content yet.
Delete Branch "dkirillov/frostfs-sdk-go:feature/73-add_pool_for_tree_endpoints"
Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?
close #73
e7db3c80e1
tof9237c2755
WIP: [#73] pool/tree: Support pool for tree serviceto [#73] pool/tree: Support pool for tree service@ -0,0 +76,4 @@
c.mu.RLock()
healthy := c.healthy
dialed := c.conn != nil
c.mu.RUnlock()
It would look a lot easier with
defer
.I tried to minimize the critical section as much as possible. But probably in this particular case overhead isn't significant (if any). I'll fix it
@ -0,0 +186,4 @@
for i, nodes := range p.rebalanceParams.nodesGroup {
clients := make([]client, len(nodes))
for j, node := range nodes {
clients[j] = newTreeClient(node.Address(), grpc.WithTransportCredentials(insecure.NewCredentials()))
Since this is a public SDK, we need to provide the user with the ability to transmit
[]gprs.DialOption
There is an separate issue for that #84
@ -0,0 +205,4 @@
}
ctx, cancel := context.WithCancel(ctx)
p.cancel = cancel
Where is
p.cancel
used?Forgot to add
Pool.Close
method@ -0,0 +206,4 @@
ctx, cancel := context.WithCancel(ctx)
p.cancel = cancel
p.closedCh = make(chan struct{})
Who is
p.closeCh
listener?f9237c2755
to1c6f88ee02
@ -0,0 +96,4 @@
func (c *treeClient) setHealthy(val bool) {
c.mu.Lock()
c.healthy = val
c.mu.Unlock()
Maybe defer here too like in all other functions in this file?
@ -0,0 +75,4 @@
logger *zap.Logger
startIndicesMtx sync.RWMutex
startIndices [2]int
It would be nice to provide some comment to the next developer, what those indices are, why there are only two of them, etc. Pool component may be really confusing sometimes, so we will benefit from some implementation details in comments.
@ -0,0 +534,4 @@
for _, cl := range group.clients {
if closeErr := cl.close(); closeErr != nil {
p.log(zapcore.ErrorLevel, "close client connection", zap.Error(closeErr))
err = closeErr
I wonder if we can go for
errors.Join
in go1.20 here 🤔With go1.20 we can write:
@ -0,0 +720,4 @@
err = fn(cl)
}
if !shouldTryAgain(err) {
p.setStartIndices(indexI, indexJ)
setStartInidices
will be invoked on every successful response and it is going to lock mutex every time. Can we do better here? Maybe update indices only ifindexI
andindexJ
have changed in this loop?@ -0,0 +6,4 @@
)
func (p *Pool) signData(buf []byte, f func(key, sign []byte)) error {
// When SDK library adopts Tree service client, this should be dropped.
Well, it is adopting now 😃
1c6f88ee02
to261580c71e
261580c71e
to19adb4dffa