Skip to content

Commit

Permalink
fix: added some disable command additions to the rename-command (#2540)
Browse files Browse the repository at this point in the history
* Added some disable command additions to the rename-command

* add rename replication test

---------

Co-authored-by: wuxianrong <[email protected]>
  • Loading branch information
Mixficsol and wuxianrong authored Mar 21, 2024
1 parent 3d62cd6 commit c21abcf
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 32 deletions.
11 changes: 6 additions & 5 deletions conf/pika.conf
Original file line number Diff line number Diff line change
Expand Up @@ -513,18 +513,19 @@ cache-lfu-decay-time: 1
#
# aclfile : ../conf/users.acl

# (experimental)
# It is possible to change the name of dangerous commands in a shared environment.
# For instance the CONFIG command may be renamed into something Warning: To prevent
# data inconsistency caused by different configuration files, do not use the rename
# command to modify write commands on the primary and secondary servers. If necessary,
# ensure that the configuration files of the primary and secondary servers are consistent
# In addition, when using the command rename, you must not use "" to modify the command,
# for example, rename-command: FLUSHALL "360flushall" is incorrect; instead, use
# rename-command: FLUSHALL 360flushall is correct. After the rename command is executed,
# for example, rename-command: FLUSHDB "360flushdb" is incorrect; instead, use
# rename-command: FLUSHDB 360flushdb is correct. After the rename command is executed,
# it is most appropriate to use a numeric string with uppercase or lowercase letters
# for example: rename-command : FLUSHALL joYAPNXRPmcarcR4ZDgC81TbdkSmLAzRPmcarcR
# for example: rename-command : FLUSHDB joYAPNXRPmcarcR4ZDgC81TbdkSmLAzRPmcarcR
# Warning: Currently only applies to flushdb, slaveof, bgsave, shutdown, config command
# Warning: Ensure that the Settings of rename-command on the master and slave servers are consistent
#
# Example:
#
# rename-command : FLUSHALL 360flushall
# rename-command : FLUSHDB 360flushdb
18 changes: 11 additions & 7 deletions tests/integration/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ import (
)

const (
LOCALHOST = "127.0.0.1"
SLAVEPORT = "9231"
MASTERPORT = "9241"
SINGLEADDR = "127.0.0.1:9221"
SLAVEADDR = "127.0.0.1:9231"
MASTERADDR = "127.0.0.1:9241"
RenameADDR = "127.0.0.1:9251"
LOCALHOST = "127.0.0.1"
SLAVEPORT = "9231"
MASTERPORT = "9241"
SLAVERENAMEPORT = "9301"
MASTERRENAMEPORT = "9291"
SINGLEADDR = "127.0.0.1:9221"
SLAVEADDR = "127.0.0.1:9231"
MASTERADDR = "127.0.0.1:9241"
SLAVERENAMEADDR = "127.0.0.1:9301"
MASTERRENAMEADDR = "127.0.0.1:9291"
RenameADDR = "127.0.0.1:9251"

CODISADDR = "127.0.0.1:19000"

Expand Down
19 changes: 0 additions & 19 deletions tests/integration/renamecommand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,4 @@ var _ = Describe("Rename Command test", func() {
r = client.Do(ctx, "flushdb")
Expect(r.Val()).NotTo(Equal("OK"))
})

It("should 360FlushAll", func() {
set := client.Set(ctx, "key", "foobar", 0)
Expect(set.Err()).NotTo(HaveOccurred())
Expect(set.Val()).To(Equal("OK"))

bitCount := client.BitCount(ctx, "key", nil)
Expect(bitCount.Err()).NotTo(HaveOccurred())
Expect(bitCount.Val()).To(Equal(int64(26)))
_, err := client.Do(ctx, "360flushall").Result()
Expect(err).NotTo(HaveOccurred())
r := client.Do(ctx, "360flushall")
Expect(r.Val()).To(Equal("OK"))
n, err := client.Exists(ctx, "key").Result()
Expect(err).NotTo(HaveOccurred())
Expect(n).To(Equal(int64(0)))
r = client.Do(ctx, "flushall")
Expect(r.Val()).NotTo(Equal("OK"))
})
})
88 changes: 88 additions & 0 deletions tests/integration/replication_rename_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package pika_integration

import (
"context"
"log"
"time"

. "github.com/bsm/ginkgo/v2"
. "github.com/bsm/gomega"
"github.com/redis/go-redis/v9"
)

var _ = Describe("should replication rename", func() {
Describe("all replication rename test", func() {
ctx := context.TODO()
var clientSlave *redis.Client
var clientMaster *redis.Client

BeforeEach(func() {
clientMaster = redis.NewClient(PikaOption(MASTERRENAMEADDR))
clientSlave = redis.NewClient(PikaOption(SLAVERENAMEADDR))
cleanEnv(ctx, clientMaster, clientSlave)
if GlobalBefore != nil {
GlobalBefore(ctx, clientMaster)
GlobalBefore(ctx, clientSlave)
}
})
AfterEach(func() {
cleanEnv(ctx, clientMaster, clientSlave)
Expect(clientSlave.Close()).NotTo(HaveOccurred())
Expect(clientMaster.Close()).NotTo(HaveOccurred())
log.Println("Replication test case done")
})

It("Let The slave become a replica of The master ", func() {
infoRes := clientSlave.Info(ctx, "replication")
Expect(infoRes.Err()).NotTo(HaveOccurred())
Expect(infoRes.Val()).To(ContainSubstring("role:master"))
infoRes = clientMaster.Info(ctx, "replication")
Expect(infoRes.Err()).NotTo(HaveOccurred())
Expect(infoRes.Val()).To(ContainSubstring("role:master"))
Expect(clientSlave.Do(ctx, "slaveof", LOCALHOST, SLAVERENAMEPORT).Err()).To(MatchError("ERR The master ip:port and the slave ip:port are the same"))

var count = 0
for {
res := trySlave(ctx, clientSlave, LOCALHOST, MASTERRENAMEPORT)
if res {
break
} else if count > 4 {
break
} else {
cleanEnv(ctx, clientMaster, clientSlave)
count++
}
}

infoRes = clientSlave.Info(ctx, "replication")
Expect(infoRes.Err()).NotTo(HaveOccurred())
Expect(infoRes.Val()).To(ContainSubstring("master_link_status:up"))

infoRes = clientMaster.Info(ctx, "replication")
Expect(infoRes.Err()).NotTo(HaveOccurred())
Expect(infoRes.Val()).To(ContainSubstring("connected_slaves:1"))

slaveWrite := clientSlave.Set(ctx, "foo", "bar", 0)
Expect(slaveWrite.Err()).To(MatchError("ERR Server in read-only"))
log.Println("Replication rename test 1 start")
set := clientMaster.Set(ctx, "x", "y", 0)
Expect(set.Err()).NotTo(HaveOccurred())
Expect(set.Val()).To(Equal("OK"))
set1 := clientMaster.Set(ctx, "a", "b", 0)
Expect(set1.Err()).NotTo(HaveOccurred())
Expect(set1.Val()).To(Equal("OK"))
r1 := clientMaster.Do(ctx, "flushdb")
Expect(r1.Val()).NotTo(Equal("OK"))
time.Sleep(3 * time.Second)
Expect(clientMaster.Do(ctx, "360flushdb").Err()).NotTo(HaveOccurred())
Eventually(func() error {
return clientMaster.Get(ctx, "x").Err()
}, "60s", "100ms").Should(Equal(redis.Nil))
Eventually(func() error {
return clientSlave.Get(ctx, "x").Err()
}, "60s", "100ms").Should(Equal(redis.Nil))
log.Println("Replication rename test 1 success")

})
})
})
26 changes: 25 additions & 1 deletion tests/integration/start_master_and_slave.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ cp ../conf/pika.conf ./pika_single.conf
cp ../conf/pika.conf ./pika_master.conf
cp ../conf/pika.conf ./pika_slave.conf
cp ../conf/pika.conf ./pika_rename.conf
cp ../conf/pika.conf ./pika_master_rename.conf
cp ../conf/pika.conf ./pika_slave_rename.conf
cp ../conf/pika.conf ./pika_acl_both_password.conf
cp ../conf/pika.conf ./pika_acl_only_admin_password.conf
cp ../conf/pika.conf ./pika_has_other_acl_user.conf
Expand Down Expand Up @@ -38,7 +40,6 @@ sed -i '' \
-e 's|#daemonize : yes|daemonize : yes|' ./pika_slave.conf

sed -i '' \
-e 's|# rename-command : FLUSHALL 360flushall|rename-command : FLUSHALL 360flushall|' \
-e 's|# rename-command : FLUSHDB 360flushdb|rename-command : FLUSHDB 360flushdb|' \
-e 's|databases : 1|databases : 2|' \
-e 's|port : 9221|port : 9251|' \
Expand Down Expand Up @@ -73,6 +74,7 @@ sed -i '' \
-e 's|pidfile : ./pika.pid|pidfile : ./acl2_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./acl2_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' ./pika_acl_only_admin_password.conf

sed -i '' \
-e 's|requirepass :|requirepass : requirepass|' \
-e 's|masterauth :|masterauth : requirepass|' \
Expand All @@ -87,6 +89,26 @@ sed -i '' \
-e 's|#daemonize : yes|daemonize : yes|' ./pika_has_other_acl_user.conf
echo -e '\nuser : limit on >limitpass ~* +@all &*' >> ./pika_has_other_acl_user.conf

sed -i '' \
-e 's|# rename-command : FLUSHDB 360flushdb|rename-command : FLUSHDB 360flushdb|' \
-e 's|port : 9221|port : 9291|' \
-e 's|log-path : ./log/|log-path : ./master_rename_data/log/|' \
-e 's|db-path : ./db/|db-path : ./master_rename_data/db/|' \
-e 's|dump-path : ./dump/|dump-path : ./master_rename_data/dump/|' \
-e 's|pidfile : ./pika.pid|pidfile : ./master_rename_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./master_rename_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' ./pika_master_rename.conf

sed -i '' \
-e 's|# rename-command : FLUSHDB 360flushdb|rename-command : FLUSHDB 360flushdb|' \
-e 's|port : 9221|port : 9301|' \
-e 's|log-path : ./log/|log-path : ./slave_rename_data/log/|' \
-e 's|db-path : ./db/|db-path : ./slave_rename_data/db/|' \
-e 's|dump-path : ./dump/|dump-path : ./slave_rename_data/dump/|' \
-e 's|pidfile : ./pika.pid|pidfile : ./slave_rename_data/pika.pid|' \
-e 's|db-sync-path : ./dbsync/|db-sync-path : ./slave_rename_data/dbsync/|' \
-e 's|#daemonize : yes|daemonize : yes|' ./pika_slave_rename.conf

# Start three nodes
./pika -c ./pika_single.conf
./pika -c ./pika_master.conf
Expand All @@ -95,5 +117,7 @@ echo -e '\nuser : limit on >limitpass ~* +@all &*' >> ./pika_has_other_acl_user.
./pika -c ./pika_acl_both_password.conf
./pika -c ./pika_acl_only_admin_password.conf
./pika -c ./pika_has_other_acl_user.conf
./pika -c ./pika_master_rename.conf
./pika -c ./pika_slave_rename.conf
#ensure both master and slave are ready
sleep 10

0 comments on commit c21abcf

Please sign in to comment.