Skip to content

Commit

Permalink
resolve #88
Browse files Browse the repository at this point in the history
* fix decoding fields for StorageCallVShardError (MasterUUID, ReplicasetUUID).
* add ReplicaUUID to the StorageCallVShardError struct.
* use constants for vshard error names.
* add comment why and how we handle "NON_MASTER" vshard error.
  • Loading branch information
nurzhan-saktaganov committed Oct 29, 2024
1 parent dd210df commit 275893e
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 237 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,29 @@

CHANGES:
* We don't support LogProvider interface anymore, only LogfProvider should be used.
* Add comment why and how we handle "NON_MASTER" vshard error.
* Don't support 'type Error struct' anymore.

BUG FIXES:
* RouterCallImpl: retry on BucketResolve error.
* RouterCallImpl: do not retry on vshard error "TRANSFER_IS_IN_PROGRESS".
* RouterCallImpl: remove misleading RetryOnCall.
* AddInstance bugfix: pass r.cfg.PoolOpts to new instance.
* Fix decoding fields for StorageCallVShardError (MasterUUID, ReplicasetUUID).

FEATURES:

* Support StdoutLoggerf that allows control log level (resolve issue #84).
* Support new BucketsSearchMode config to set policy for BucketDiscovery (resolve #71).
* Implemented CalculateEtalonBalance based on lua router (part of #32).
* Add ReplicaUUID to the StorageCallVShardError struct.

REFACTOR:

* Func bucketSearchLegacy: log error from bucketStatWait (except bucketStatError).
* New bucketsDiscoveryAsync, bucketsDiscoveryWait, bucketsDiscovery methods for buckets discovery pagination.
* Support bucketSearchBatched method for batched buckets discovery (resolve #71).
* Use constants for vshard error names and codes.

TESTS:
* Tests for BucketsSearchMode (tnt/discovery_test.go).
Expand Down
31 changes: 18 additions & 13 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,17 @@ func (s storageCallAssertError) Error() string {
}

type StorageCallVShardError struct {
BucketID uint64 `msgpack:"bucket_id" mapstructure:"bucket_id"`
Reason string `msgpack:"reason"`
Code int `msgpack:"code"`
Type string `msgpack:"type"`
Message string `msgpack:"message"`
Name string `msgpack:"name"`
MasterUUID *string `msgpack:"master_uuid" mapstructure:"master_uuid"` // mapstructure cant decode to source uuid type
ReplicasetUUID *string `msgpack:"replicaset_uuid" mapstructure:"replicaset_uuid"` // mapstructure cant decode to source uuid type
BucketID uint64 `msgpack:"bucket_id" mapstructure:"bucket_id"`
Reason string `msgpack:"reason"`
Code int `msgpack:"code"`
Type string `msgpack:"type"`
Message string `msgpack:"message"`
Name string `msgpack:"name"`
// These 3 fields below are send as string by vshard storage, so we decode them into string, not uuid.UUID type
// Example: 00000000-0000-0002-0002-000000000000
MasterUUID string `msgpack:"master" mapstructure:"master"`
ReplicasetUUID string `msgpack:"replicaset" mapstructure:"replicaset"`
ReplicaUUID string `msgpack:"replica" mapstructure:"replica"`
}

func (s StorageCallVShardError) Error() string {
Expand Down Expand Up @@ -165,7 +168,7 @@ func (r *Router) RouterCallImpl(ctx context.Context,
}

switch vshardError.Name {
case "WRONG_BUCKET", "BUCKET_IS_LOCKED":
case VShardErrNameWrongBucket, VShardErrNameBucketIsLocked:
r.BucketReset(bucketID)

// TODO we should inspect here err.destination like lua vshard router does,
Expand All @@ -179,15 +182,17 @@ func (r *Router) RouterCallImpl(ctx context.Context,
// this vshardError will be returned to a caller in case of timeout
err = &vshardError
continue
case "TRANSFER_IS_IN_PROGRESS":
case VShardErrNameTransferIsInProgress:
// Since lua vshard router doesn't retry here, we don't retry too.
// There is a comment why lua vshard router doesn't retry:
// https://github.com/tarantool/vshard/blob/b6fdbe950a2e4557f05b83bd8b846b126ec3724e/vshard/router/init.lua#L697
r.BucketReset(bucketID)
return nil, nil, &vshardError
case "NON_MASTER":
// We don't know how to handle this case yet, so just return it for now.
// Here is issue for it: https://github.com/KaymeKaydex/go-vshard-router/issues/88
case VShardErrNameNonMaster:
// vshard.storage has returned NON_MASTER error, lua vshard router updates info about master in this case:
// See: https://github.com/tarantool/vshard/blob/b6fdbe950a2e4557f05b83bd8b846b126ec3724e/vshard/router/init.lua#L704.
// Since we use go-tarantool library, and go-tarantool library doesn't provide API to update info about current master,
// we just return this error as is.
return nil, nil, &vshardError
default:
return nil, nil, &vshardError
Expand Down
6 changes: 3 additions & 3 deletions discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (r *Router) bucketSearchLegacy(ctx context.Context, bucketID uint64) (*Repl
rs, err := r.BucketSet(bucketID, rsFuture.rsID)
if err != nil {
r.log().Errorf(ctx, "bucketSearchLegacy: can't set rsID %v for bucketID %d: %v", rsFuture.rsID, bucketID, err)
return nil, Errors[9] // NO_ROUTE_TO_BUCKET
return nil, newVShardErrorNoRouteToBucket(bucketID)
}

// TODO: should we release resources for unhandled futures?
Expand All @@ -116,7 +116,7 @@ func (r *Router) bucketSearchLegacy(ctx context.Context, bucketID uint64) (*Repl
-- discovery).
*/

return nil, Errors[9] // NO_ROUTE_TO_BUCKET
return nil, newVShardErrorNoRouteToBucket(bucketID)
}

// The approach in bucketSearchLegacy is very ineffective because
Expand Down Expand Up @@ -177,7 +177,7 @@ func (r *Router) bucketSearchBatched(ctx context.Context, bucketIDToFind uint64)
}

if rs == nil {
return nil, Errors[9] // NO_ROUTE_TO_BUCKET
return nil, newVShardErrorNoRouteToBucket(bucketIDToFind)
}

return rs, nil
Expand Down
Loading

0 comments on commit 275893e

Please sign in to comment.