Skip to content

Commit f2929a9

Browse files
feat: reorder rpcs if they fail all retries (#132)
## feat: reorder rpcs if they fail all retries Currently if the default RPC (or any other RPC) starts failing after the client initialisation the multi-client will still keep the same order for RPCs possibly using the same bad RPC on the next call. This change reorders the RPCs and changes the default RPC when a request failed in a way that all RPCs that failed will go down on the RPC's backup list and the successful RPC will become the primary. If the primary RPC was successful or there are no backups, no re-ordering will be performed. ### Test Durable Pipeline: https://github.com/smartcontractkit/chainlink-deployments/actions/runs/15415638318 ___ Issue:[ CLD-261](https://smartcontract-it.atlassian.net/browse/CLD-261)
1 parent 4ed7e21 commit f2929a9

3 files changed

Lines changed: 149 additions & 5 deletions

File tree

.changeset/ready-items-dance.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"chainlink-deployments-framework": minor
3+
---
4+
5+
feat: reorder bad RPCs if they fail all retries

deployment/multiclient.go

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"math/big"
8+
"sync"
89
"time"
910

1011
"github.com/avast/retry-go/v4"
@@ -66,6 +67,7 @@ type MultiClient struct {
6667
RetryConfig RetryConfig
6768
lggr logger.Logger
6869
chainName string
70+
mu sync.RWMutex
6971
}
7072

7173
// rpcHealthCheck performs a basic health check on the RPC client by calling eth_blockNumber
@@ -334,7 +336,7 @@ func (mc *MultiClient) WaitMined(ctx context.Context, tx *types.Transaction) (*t
334336
}
335337
}
336338

337-
for _, client := range append([]*ethclient.Client{mc.Client}, mc.Backups...) {
339+
for _, client := range mc.clients() {
338340
go waitMined(client, tx)
339341
}
340342
var receipt *types.Receipt
@@ -355,29 +357,33 @@ func (mc *MultiClient) WaitMined(ctx context.Context, tx *types.Transaction) (*t
355357
func (mc *MultiClient) retryWithBackups(ctx context.Context, opName string, op func(context.Context, *ethclient.Client) error) error {
356358
var err error
357359
traceID := uuid.New()
358-
for i, client := range append([]*ethclient.Client{mc.Client}, mc.Backups...) {
360+
361+
for rpcIndex, client := range mc.clients() {
359362
retryCount := 0
360363
err2 := retry.Do(func() error {
361364
timeoutCtx, cancel := ensureTimeout(ctx, mc.RetryConfig.Timeout)
362365
defer cancel()
363366

364367
err = op(timeoutCtx, client)
365368
if err != nil {
366-
mc.lggr.Warnf("traceID %q: chain %q: op: %q: client index %d: failed execution - retryable error '%s'", traceID.String(), mc.chainName, opName, i, MaybeDataErr(err))
369+
mc.lggr.Warnf("traceID %q: chain %q: op: %q: client index %d: failed execution - retryable error '%s'", traceID.String(), mc.chainName, opName, rpcIndex, MaybeDataErr(err))
367370
return err
368371
}
369372

373+
// If the operation was successful, check if we need to reorder the RPCs
374+
mc.reorderRPCs(rpcIndex)
375+
370376
return nil
371377
}, retry.Attempts(mc.RetryConfig.Attempts), retry.Delay(mc.RetryConfig.Delay),
372378
retry.OnRetry(func(n uint, err error) { retryCount++ }))
373379
if err2 == nil {
374380
if retryCount > 0 {
375-
mc.lggr.Infof("traceID %q: chain %q: op: %q: client index %d: successfully executed after %d retry", traceID.String(), mc.chainName, opName, i, retryCount)
381+
mc.lggr.Infof("traceID %q: chain %q: op: %q: client index %d: successfully executed after %d retry", traceID.String(), mc.chainName, opName, rpcIndex, retryCount)
376382
}
377383

378384
return nil
379385
}
380-
mc.lggr.Infof("traceID %q: chain %q: op: %q: client index %d: failed, trying next client", traceID.String(), mc.chainName, opName, i)
386+
mc.lggr.Infof("traceID %q: chain %q: op: %q: client index %d: failed, trying next client", traceID.String(), mc.chainName, opName, rpcIndex)
381387
}
382388

383389
return errors.Join(err, fmt.Errorf("all backup clients failed for chain %q", mc.chainName))
@@ -431,3 +437,36 @@ func ensureTimeout(parent context.Context, timeout time.Duration) (context.Conte
431437
// create a new context with the specified timeout
432438
return context.WithTimeout(parent, timeout)
433439
}
440+
441+
// reorderRPCs reorders the RPCs based on the latest call.
442+
// If the default RPC failed all attempts, it will be moved to the end of the backup list.
443+
// If backup RPCs also failed, they will be moved to the end of the backup list.
444+
// If the primary RPC worked, it will remain the first in the list.
445+
func (mc *MultiClient) reorderRPCs(rpcIndex int) {
446+
mc.mu.Lock()
447+
defer mc.mu.Unlock()
448+
449+
if rpcIndex < 1 || len(mc.Backups) == 0 {
450+
return // No need to reorder if the first RPC is still the default or we don't have backups
451+
}
452+
453+
// Find the index of the backupRPC
454+
newDefaultRPCIndex := rpcIndex - 1
455+
newDefaultRPC := mc.Backups[newDefaultRPCIndex]
456+
457+
// Reorder the failed backups to the end of the list
458+
reordered := make([]*ethclient.Client, 0, len(mc.Backups))
459+
reordered = append(reordered, mc.Backups[newDefaultRPCIndex+1:]...)
460+
reordered = append(reordered, mc.Backups[:newDefaultRPCIndex]...)
461+
reordered = append(reordered, mc.Client)
462+
463+
mc.Backups = reordered
464+
mc.Client = newDefaultRPC
465+
}
466+
467+
func (mc *MultiClient) clients() []*ethclient.Client {
468+
mc.mu.RLock()
469+
defer mc.mu.RUnlock()
470+
471+
return append([]*ethclient.Client{mc.Client}, mc.Backups...)
472+
}

deployment/multiclient_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,103 @@ func TestEnsureTimeout(t *testing.T) {
295295
})
296296
}
297297
}
298+
func TestMultiClient_reorderRPCs(t *testing.T) {
299+
t.Parallel()
300+
301+
// Create some test clients with different memory addresses for identification
302+
client0 := ethclient.NewClient(nil) // primary
303+
client1 := ethclient.NewClient(nil) // backup 0
304+
client2 := ethclient.NewClient(nil) // backup 1
305+
client3 := ethclient.NewClient(nil) // backup 2
306+
307+
rpcClients := []*ethclient.Client{
308+
client1, // backup 0
309+
client2, // backup 1
310+
client3, // backup 2
311+
}
312+
313+
tests := []struct {
314+
name string
315+
backups []*ethclient.Client
316+
newDefaultClientIdx int
317+
expectedClient *ethclient.Client
318+
expectedBackups []*ethclient.Client
319+
}{
320+
{
321+
name: "Move first backup to primary",
322+
backups: rpcClients,
323+
newDefaultClientIdx: 1,
324+
expectedClient: client1,
325+
expectedBackups: []*ethclient.Client{
326+
client2,
327+
client3,
328+
client0,
329+
},
330+
},
331+
{
332+
name: "Move middle backup to primary",
333+
backups: rpcClients,
334+
newDefaultClientIdx: 2,
335+
expectedClient: client2,
336+
expectedBackups: []*ethclient.Client{
337+
client3,
338+
client1,
339+
client0,
340+
},
341+
},
342+
{
343+
name: "Move last backup to primary",
344+
backups: rpcClients,
345+
newDefaultClientIdx: 3,
346+
expectedClient: client3,
347+
expectedBackups: []*ethclient.Client{
348+
client1,
349+
client2,
350+
client0,
351+
},
352+
},
353+
{
354+
name: "Keep primary unchanged",
355+
backups: rpcClients,
356+
newDefaultClientIdx: 0,
357+
expectedClient: client0,
358+
expectedBackups: []*ethclient.Client{
359+
client1,
360+
client2,
361+
client3,
362+
},
363+
},
364+
{
365+
name: "Keep primary unchanged when no backups",
366+
backups: []*ethclient.Client{},
367+
newDefaultClientIdx: 1,
368+
expectedClient: client0,
369+
expectedBackups: []*ethclient.Client{},
370+
},
371+
}
372+
373+
for _, tt := range tests {
374+
t.Run(tt.name, func(t *testing.T) {
375+
t.Parallel()
376+
377+
mc := &MultiClient{
378+
Client: client0,
379+
Backups: tt.backups,
380+
lggr: logger.Test(t),
381+
}
382+
383+
// Call the method being tested
384+
mc.reorderRPCs(tt.newDefaultClientIdx)
385+
386+
// Verify the results
387+
assert.Same(t, tt.expectedClient, mc.Client, "Primary client should be the selected backup")
388+
require.Len(t, mc.Backups, len(tt.expectedBackups), "Backup count should remain the same")
389+
390+
// Check that backups are in the expected order
391+
for i, expected := range tt.expectedBackups {
392+
assert.Same(t, expected, mc.Backups[i],
393+
"Backup at position %d should be as expected", i)
394+
}
395+
})
396+
}
397+
}

0 commit comments

Comments
 (0)