resolve #88

* 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.

Author: nurzhan-saktaganov

CHANGELOG.md

@@ -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).

api.go

@@ -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 {
@@ -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,
@@ -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

discovery.go

@@ -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?
@@ -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
@@ -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