morph: Iterate over endpoints when create ws client in constructor #307
3 changed files with 24 additions and 3 deletions
|
@ -13,6 +13,7 @@ Changelog for FrostFS Node
|
|||
- Read config files from dir even if config file not provided via `--config` for node (#238)
|
||||
- Notary requests parsing according to `neo-go`'s updates (#268)
|
||||
- Tree service panic in its internal client cache (#322)
|
||||
- Iterate over endpoints when create ws client in morph's constructor (#304)
|
||||
|
||||
### Removed
|
||||
### Updated
|
||||
|
|
|
@ -413,6 +413,8 @@ const (
|
|||
FrostFSIRInternalError = "internal error" // Info in ../node/cmd/frostfs-ir/main.go
|
||||
FrostFSIRCouldNotShutdownHTTPServer = "could not shutdown HTTP server" // Debug in ../node/cmd/frostfs-ir/main.go
|
||||
FrostFSIRApplicationStopped = "application stopped" // Info in ../node/cmd/frostfs-ir/main.go
|
||||
FrostFSIRCouldntCreateRPCClientForEndpoint = "could not create RPC client for endpoint" // Debug in ../node/pkg/morph/client/constructor.go
|
||||
FrostFSIRCreatedRPCClientForEndpoint = "created RPC client for endpoint" // Info in ../node/pkg/morph/client/constructor.go
|
||||
FrostFSNodeCouldNotReadCertificateFromFile = "could not read certificate from file" // Error in ../node/cmd/frostfs-node/grpc.go
|
||||
FrostFSNodeCantListenGRPCEndpoint = "can't listen gRPC endpoint" // Error in ../node/cmd/frostfs-node/grpc.go
|
||||
FrostFSNodeStopListeningGRPCEndpoint = "stop listening gRPC endpoint" // Info in ../node/cmd/frostfs-node/grpc.go
|
||||
|
|
|
@ -7,6 +7,7 @@ import (
|
|||
"sync"
|
||||
"time"
|
||||
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger"
|
||||
lru "github.com/hashicorp/golang-lru/v2"
|
||||
"github.com/nspcc-dev/neo-go/pkg/core/block"
|
||||
|
@ -52,6 +53,10 @@ const (
|
|||
defaultWaitInterval = 500 * time.Millisecond
|
||||
)
|
||||
|
||||
var (
|
||||
|
||||
ErrNoHealthyEndpoint = errors.New("no healthy endpoint")
|
||||
)
|
||||
|
||||
func defaultConfig() *cfg {
|
||||
return &cfg{
|
||||
dialTimeout: defaultDialTimeout,
|
||||
|
@ -80,6 +85,8 @@ func defaultConfig() *cfg {
|
|||
// If desired option satisfies the default value, it can be omitted.
|
||||
// If multiple options of the same config value are supplied,
|
||||
// the option with the highest index in the arguments will be used.
|
||||
// If the list of endpoints provided - uses first alive.
|
||||
// If there are no healthy endpoint - returns ErrNoHealthyEndpoint.
|
||||
func New(ctx context.Context, key *keys.PrivateKey, opts ...Option) (*Client, error) {
|
||||
if key == nil {
|
||||
panic("empty private key")
|
||||
|
@ -137,9 +144,20 @@ func New(ctx context.Context, key *keys.PrivateKey, opts ...Option) (*Client, er
|
|||
return nil, fmt.Errorf("could not create RPC actor: %w", err)
|
||||
}
|
||||
fyrchik marked this conversation as resolved
Outdated
fyrchik
commented
We usually log the message and use We usually log the message and use `zap.Error`, why have you decided to `Debug(err.Error())`
acid-ant
commented
Because in this case, error already contains context. Also, to avoid duplication, cause in the worth case we need to return error. Because in this case, error already contains context. Also, to avoid duplication, cause in the worth case we need to return error.
acid-ant
commented
Updated, please review. Updated, please review.
|
||||
} else {
|
||||
fyrchik marked this conversation as resolved
Outdated
fyrchik
commented
Can we move this line out of the loop?
Can we move this line out of the loop?
The error could be defined separately -- it is not the last, but e.g. `ErrNoHealthyEndpoint`
```
var cli *Client
for { ... }
if cli == nil {
return nil, ErrNoHealthyEndpoint
}
```
acid-ant
commented
Nice suggestion, will implement it. Nice suggestion, will implement it.
acid-ant
commented
Updated, please review. Updated, please review.
|
||||
cli.client, act, err = cli.newCli(ctx, cli.endpoints.list[0].Address)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("could not create RPC client: %w", err)
|
||||
var endpoint Endpoint
|
||||
for cli.endpoints.curr, endpoint = range cli.endpoints.list {
|
||||
fyrchik marked this conversation as resolved
Outdated
fyrchik
commented
Use constant from Use constant from `internal/logs`?
acid-ant
commented
Done Done
|
||||
cli.client, act, err = cli.newCli(ctx, endpoint.Address)
|
||||
if err != nil {
|
||||
fyrchik marked this conversation as resolved
Outdated
fyrchik
commented
Again, what about Again, what about `zap.String("endpoint", endpoint.Address)`?
acid-ant
commented
Sorry, flashbacks from java :) Sorry, flashbacks from java :)
acid-ant
commented
Updated, please review. Updated, please review.
|
||||
cli.logger.Warn(logs.FrostFSIRCouldntCreateRPCClientForEndpoint,
|
||||
dstepanov-yadro
commented
Maybe Warn, not Debug? Maybe Warn, not Debug?
acid-ant
commented
Why not, updated. Why not, updated.
|
||||
zap.Error(err), zap.String("endpoint", endpoint.Address))
|
||||
} else {
|
||||
cli.logger.Info(logs.FrostFSIRCreatedRPCClientForEndpoint,
|
||||
zap.String("endpoint", endpoint.Address))
|
||||
break
|
||||
}
|
||||
}
|
||||
if cli.client == nil {
|
||||
return nil, ErrNoHealthyEndpoint
|
||||
}
|
||||
}
|
||||
cli.setActor(act)
|
||||
|
|
Loading…
Add table
Reference in a new issue
Does linter say nothing here?
Maybe add some comment to public errors.
Nothing, didn't disable anything related to pre-commit check.
Add more info about ErrNoHealthyEndpoint in help for New(...)