Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix:fix Zpopmaxbugs return value not same as redis #2188

Merged
merged 21 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/pika_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ class PikaServer : public pstd::noncopyable {
void AutoKeepAliveRSync();
void AutoUpdateNetworkMetric();
void PrintThreadPoolQueueStatus();

std::string host_;
int port_ = 0;
time_t start_time_s_ = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/pika_command.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ extern PikaServer* g_pika_server;
extern std::unique_ptr<PikaReplicaManager> g_pika_rm;
extern std::unique_ptr<PikaCmdTableManager> g_pika_cmd_table_manager;


hero-heng marked this conversation as resolved.
Show resolved Hide resolved
void InitCmdTable(CmdTable* cmd_table) {
// Admin
////Slaveof
Expand Down Expand Up @@ -417,7 +418,6 @@ void InitCmdTable(CmdTable* cmd_table) {
std::make_unique<LPushCmd>(kCmdNameLPush, -3, kCmdFlagsWrite | kCmdFlagsSingleSlot | kCmdFlagsList | kCmdFlagsDoThroughDB | kCmdFlagsUpdateCache);
cmd_table->insert(std::pair<std::string, std::unique_ptr<Cmd>>(kCmdNameLPush, std::move(lpushptr)));
std::unique_ptr<Cmd> lpushxptr =

hero-heng marked this conversation as resolved.
Show resolved Hide resolved
std::make_unique<LPushxCmd>(kCmdNameLPushx, -3, kCmdFlagsWrite | kCmdFlagsSingleSlot | kCmdFlagsList | kCmdFlagsDoThroughDB | kCmdFlagsUpdateCache);
std::make_unique<LPushxCmd>(kCmdNameLPushx, 3, kCmdFlagsWrite | kCmdFlagsSingleSlot | kCmdFlagsList);
cmd_table->insert(std::pair<std::string, std::unique_ptr<Cmd>>(kCmdNameLPushx, std::move(lpushxptr)));
Expand Down
18 changes: 10 additions & 8 deletions src/pika_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,11 @@ void LPopCmd::DoInitial() {
}
key_ = argv_[1];
size_t argc = argv_.size();
size_t index = 2;
if (index < argc) {
if (pstd::string2int(argv_[index].data(), argv_[index].size(), &count_) == 0) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameLPop);
if (argc > 3) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameLPop);
} else if (argc == 3) {
if (pstd::string2int(argv_[2].data(), argv_[2].size(), &count_) == 0) {
res_.SetRes(CmdRes::kErrOther, kCmdNameLPop);
return;
}
if (count_ < 0) {
Expand Down Expand Up @@ -728,10 +729,11 @@ void RPopCmd::DoInitial() {
return;
}
key_ = argv_[1];
size_t index = 2;
if (index < argv_.size()) {
if (pstd::string2int(argv_[index].data(), argv_[index].size(), &count_) == 0) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameRPop);
if (argv_.size() > 3) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameRPop);
} else if (argv_.size() == 3) {
if (pstd::string2int(argv_[2].data(), argv_[2].size(), &count_) == 0) {
res_.SetRes(CmdRes::kErrOther, kCmdNameRPop);
return;
}
if (count_ < 0) {
Expand Down
34 changes: 17 additions & 17 deletions src/pika_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ void PikaServer::SlotSetSmallCompactionDurationThreshold(uint32_t small_compacti
}

bool PikaServer::GetDBSlotBinlogOffset(const std::string& db_name, uint32_t slot_id,
BinlogOffset* const boffset) {
BinlogOffset* const boffset) {
std::shared_ptr<SyncMasterSlot> slot =
g_pika_rm->GetSyncMasterSlotByName(SlotInfo(db_name, slot_id));
if (!slot) {
Expand Down Expand Up @@ -1233,7 +1233,7 @@ void PikaServer::ResetLastSecQuerynum() {
}

void PikaServer::UpdateQueryNumAndExecCountDB(const std::string& db_name, const std::string& command,
bool is_write) {
bool is_write) {
std::string cmd(command);
statistic_.server_stat.qps.querynum++;
statistic_.server_stat.exec_count_db[pstd::StringToUpper(cmd)]++;
Expand Down Expand Up @@ -1537,13 +1537,13 @@ void PikaServer::AutoUpdateNetworkMetric() {
}

void PikaServer::PrintThreadPoolQueueStatus() {
// Print the current queue size if it exceeds QUEUE_SIZE_THRESHOLD_PERCENTAGE/100 of the maximum queue size.
size_t cur_size = ClientProcessorThreadPoolCurQueueSize();
size_t max_size = ClientProcessorThreadPoolMaxQueueSize();
size_t thread_hold = (max_size / 100) * QUEUE_SIZE_THRESHOLD_PERCENTAGE;
if (cur_size > thread_hold) {
LOG(INFO) << "The current queue size of the Pika Server's client thread processor thread pool: " << cur_size;
}
// Print the current queue size if it exceeds QUEUE_SIZE_THRESHOLD_PERCENTAGE/100 of the maximum queue size.
size_t cur_size = ClientProcessorThreadPoolCurQueueSize();
size_t max_size = ClientProcessorThreadPoolMaxQueueSize();
size_t thread_hold = (max_size / 100) * QUEUE_SIZE_THRESHOLD_PERCENTAGE;
if (cur_size > thread_hold) {
LOG(INFO) << "The current queue size of the Pika Server's client thread processor thread pool: " << cur_size;
}
}

void PikaServer::InitStorageOptions() {
Expand Down Expand Up @@ -1600,14 +1600,14 @@ void PikaServer::InitStorageOptions() {
}

storage_options_.options.rate_limiter =
std::shared_ptr<rocksdb::RateLimiter>(
rocksdb::NewGenericRateLimiter(
g_pika_conf->rate_limiter_bandwidth(),
g_pika_conf->rate_limiter_refill_period_us(),
static_cast<int32_t>(g_pika_conf->rate_limiter_fairness()),
rocksdb::RateLimiter::Mode::kWritesOnly,
g_pika_conf->rate_limiter_auto_tuned()
));
std::shared_ptr<rocksdb::RateLimiter>(
rocksdb::NewGenericRateLimiter(
g_pika_conf->rate_limiter_bandwidth(),
g_pika_conf->rate_limiter_refill_period_us(),
static_cast<int32_t>(g_pika_conf->rate_limiter_fairness()),
rocksdb::RateLimiter::Mode::kWritesOnly,
g_pika_conf->rate_limiter_auto_tuned()
));

// For Storage small compaction
storage_options_.statistics_max_size = g_pika_conf->max_cache_statistic_keys();
Expand Down
11 changes: 6 additions & 5 deletions src/pika_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,25 @@ void SAddCmd::DoUpdateCache(std::shared_ptr<Slot> slot) {

void SPopCmd::DoInitial() {
size_t argc = argv_.size();
size_t index = 2;
if (!CheckArg(argc)) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameSPop);
return;
}

key_ = argv_[1];
count_ = 1;

if (index < argc) {
if (pstd::string2int(argv_[index].data(), argv_[index].size(), &count_) == 0) {
if (argc > 3) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameSPop);
} else if (argc == 3) {
if (pstd::string2int(argv_[2].data(), argv_[2].size(), &count_) == 0) {
res_.SetRes(CmdRes::kErrOther, kCmdNameSPop);
return;
}
if (count_ <= 0) {
res_.SetRes(CmdRes::kErrOther, kCmdNameSPop);
return;
}
} else {
hero-heng marked this conversation as resolved.
Show resolved Hide resolved
count_ = 1;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/pika_slot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "include/pika_rm.h"
#include "include/pika_server.h"
#include "include/pika_slot.h"
#include "include/pika_command.h"

#include "pstd/include/mutex_impl.h"
#include "pstd/include/pstd_hash.h"
Expand Down Expand Up @@ -389,7 +390,6 @@ bool Slot::RunBgsaveEngine() {
return false;
}
LOG(INFO) << slot_name_ << " create new backup finished.";

hero-heng marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

Expand Down
26 changes: 14 additions & 12 deletions src/pika_zset.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1425,13 +1425,14 @@ void ZPopmaxCmd::DoInitial() {
return;
}
key_ = argv_[1];
if (argv_.size() == 2) {
if (argv_.size() > 3) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameZPopmax);
} else if (argv_.size() == 3) {
if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
res_.SetRes(CmdRes::kInvalidInt);
}
} else {
hero-heng marked this conversation as resolved.
Show resolved Hide resolved
count_ = 1;
return;
}
if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
res_.SetRes(CmdRes::kInvalidInt);
return;
}
}

Expand Down Expand Up @@ -1459,13 +1460,14 @@ void ZPopminCmd::DoInitial() {
return;
}
key_ = argv_[1];
if (argv_.size() == 2) {
if (argv_.size() > 3) {
res_.SetRes(CmdRes::kWrongNum, kCmdNameZPopmin);
} else if (argv_.size() == 3) {
if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
res_.SetRes(CmdRes::kInvalidInt);
}
} else {
hero-heng marked this conversation as resolved.
Show resolved Hide resolved
count_ = 1;
return;
}
if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
res_.SetRes(CmdRes::kInvalidInt);
return;
}
}

Expand Down
6 changes: 6 additions & 0 deletions tests/integration/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ var _ = Describe("List Commands", func() {
lRange := client.LRange(ctx, "list", 0, -1)
Expect(lRange.Err()).NotTo(HaveOccurred())
Expect(lRange.Val()).To(Equal([]string{"two", "three"}))

err := client.Do(ctx, "LPOP", "list", 1, 2).Err()
Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'lpop' command")))
hero-heng marked this conversation as resolved.
Show resolved Hide resolved
})

It("should LPopCount", func() {
Expand Down Expand Up @@ -631,6 +634,9 @@ var _ = Describe("List Commands", func() {
lRange := client.LRange(ctx, "list", 0, -1)
Expect(lRange.Err()).NotTo(HaveOccurred())
Expect(lRange.Val()).To(Equal([]string{"one", "two"}))

err := client.Do(ctx, "RPOP", "list", 1, 2).Err()
hero-heng marked this conversation as resolved.
Show resolved Hide resolved
Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'rpop' command")))
})

It("should RPopCount", func() {
Expand Down
28 changes: 23 additions & 5 deletions tests/integration/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,11 +390,29 @@ var _ = Describe("Server", func() {
// Expect(info.Val()).To(ContainSubstring(`memory`))
//})
//
//It("should LastSave", func() {
// lastSave := client.LastSave(ctx)
// Expect(lastSave.Err()).NotTo(HaveOccurred())
// Expect(lastSave.Val()).NotTo(Equal(0))
//})
// It("should LastSave", func() {
// lastSave := client.LastSave(ctx)
// Expect(lastSave.Err()).NotTo(HaveOccurred())
// Expect(lastSave.Val()).NotTo(Equal(0))
//
// mset := client.MSet(ctx, "key", "Hello", "key1", "Hello1")
// Expect(mset.Err()).NotTo(HaveOccurred())
// Expect(mset.Val()).NotTo(Equal("OK"))
//
// mGet := client.MGet(ctx, "key", "key1")
// Expect(mGet.Err()).NotTo(HaveOccurred())
// Expect(mGet.Val()).To(Equal([]interface{}{"Hello", "Hello1"}))
//
// bgSave, err := client.BgSave(ctx).Result()
// bgSaveTime := time.Now().Unix()
// Expect(err).NotTo(HaveOccurred())
// Expect(bgSave).To(ContainSubstring("Background saving started"))
//
// lastSave = client.LastSave(ctx)
// Expect(lastSave.Err()).NotTo(HaveOccurred())
// Expect(lastSave.Val()).NotTo(Equal(int64(bgSaveTime)))
// // Missing lastsave persistence test
// })

//It("should Save", func() {
//
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,9 @@ var _ = Describe("Set Commands", func() {
sMembers := client.SMembers(ctx, "set")
Expect(sMembers.Err()).NotTo(HaveOccurred())
Expect(sMembers.Val()).To(HaveLen(3))

err := client.Do(ctx, "SPOP", "set", 1, 2).Err()
hero-heng marked this conversation as resolved.
Show resolved Hide resolved
Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'spop' command")))
})

It("should SPopN", func() {
Expand Down
4 changes: 4 additions & 0 deletions tests/integration/zset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,8 @@ var _ = Describe("Zset Commands", func() {
Score: 1,
Member: "one",
}}))
err = client.Do(ctx, "ZPOPMAX", "zset", 1, 2).Err()
Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'zpopmax' command")))
})

It("should ZPopMin", func() {
Expand Down Expand Up @@ -1083,6 +1085,8 @@ var _ = Describe("Zset Commands", func() {
Score: 3,
Member: "three",
}}))
err = client.Do(ctx, "ZPOPMIN", "zset", 1, 2).Err()
Expect(err).To(MatchError(ContainSubstring("ERR wrong number of arguments for 'zpopmin' command")))
hero-heng marked this conversation as resolved.
Show resolved Hide resolved
})

It("should ZRange", func() {
Expand Down
Loading