ellutionist commented on code in PR #1464:
URL:
https://github.com/apache/incubator-kvrocks/pull/1464#discussion_r1202324820
##########
tests/gocase/integration/slotmigrate/slotmigrate_test.go:
##########
@@ -413,22 +413,18 @@ func TestSlotMigrateSync(t *testing.T) {
t.Run("MIGRATE - Migrate sync with (or without) all kinds of timeouts",
func(t *testing.T) {
slot++
- require.Equal(t, "OK", rdb0.Do(ctx, "clusterx", "migrate",
slot, id1, "sync").Val())
-
- slot++
- require.Equal(t, "OK", rdb0.Do(ctx, "clusterx", "migrate",
slot, id1, "sync", -1).Val())
-
- slot++
- require.Equal(t, "OK", rdb0.Do(ctx, "clusterx", "migrate",
slot, id1, "sync", 0).Val())
-
- slot++
- require.Equal(t, "OK", rdb0.Do(ctx, "clusterx", "migrate",
slot, id1, "sync", 10).Val())
-
- slot++
- require.Equal(t, "OK", rdb0.Do(ctx, "clusterx", "migrate",
slot, id1, "sync", 0.5).Val())
-
- slot++
- require.Equal(t, "OK", rdb0.Do(ctx, "clusterx", "migrate",
slot, id1, "sync", -3.14).Val())
+ // migrate sync without explicitly specify the timeout
+ result := rdb0.Do(ctx, "clusterx", "migrate", slot, id1, "sync")
+ require.NoError(t, result.Err())
+ require.Equal(t, "OK", result.Val())
+
+ timeouts := []interface{}{-1, 0, 20, 10.5, -3.14}
Review Comment:
@torwig @git-hulk The original reason to support float timeout is that in
the go tests I want to "deliberately let the sync migration timeout" (see
[here](https://github.com/apache/incubator-kvrocks/blob/unstable/tests/gocase/integration/slotmigrate/slotmigrate_test.go#L440)).
And the minimal integer value of "1" is too large and cannot ensure the
timeout happen...
Also I think supporting float timeout is not harmful but provides more
precise control to the migration. What do you think?
As for the negative value, returning an error when the server receives an
negative value seems acceptable to me. No problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
