Skip to content

Commit c27c63a

Browse files
committed
MB-66628: SetParam must use CAS=0 [3/8]
ns_server never implemented the support for setting the control token which means that it has always provided CAS == 0. Remove support for the control token as the list of commands supporting it isn't very consistent and a new task should be scheduled to do a full scrub over all commands if we want to implement this. Change-Id: I1d2e37620f8d26c3f3685d8a579374aefd6c4ae5 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/227526 Reviewed-by: Vesko Karaganev <vesko.karaganev@couchbase.com> Tested-by: Build Bot <build@couchbase.com>
1 parent c231b28 commit c27c63a

File tree

8 files changed

+97
-51
lines changed

8 files changed

+97
-51
lines changed

daemon/mcbp_executors.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#include "protocol/mcbp/set_active_encryption_keys_context.h"
5454
#include "protocol/mcbp/set_chronicle_auth_token_command_context.h"
5555
#include "protocol/mcbp/set_cluster_config_command_context.h"
56+
#include "protocol/mcbp/set_param_command_context.h"
5657
#include "protocol/mcbp/settings_reload_command_context.h"
5758
#include "protocol/mcbp/single_state_steppable_context.h"
5859
#include "protocol/mcbp/start_fusion_uploader_command_context.h"
@@ -576,7 +577,7 @@ static void update_user_permissions_executor(Cookie& cookie) {
576577
}
577578

578579
static void set_param_executor(Cookie& cookie) {
579-
cookie.obtainContext<SetParameterCommandContext>(cookie).drive();
580+
cookie.obtainContext<SetParamCommandContext>(cookie).drive();
580581
}
581582

582583
static void get_vbucket_executor(Cookie& cookie) {

daemon/mcbp_validators.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2353,7 +2353,7 @@ static Status set_param_validator(Cookie& cookie) {
23532353
sizeof(SetParamPayload),
23542354
ExpectedKeyLen::NonZero,
23552355
ExpectedValueLen::NonZero,
2356-
ExpectedCas::Any,
2356+
ExpectedCas::NotSet,
23572357
GeneratesDocKey::No,
23582358
PROTOCOL_BINARY_RAW_BYTES);
23592359

daemon/protocol/mcbp/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ add_library(memcached_daemon_mcbp OBJECT
106106
set_chronicle_auth_token_command_context.h
107107
set_cluster_config_command_context.cc
108108
set_cluster_config_command_context.h
109+
set_param_command_context.cc
110+
set_param_command_context.h
109111
settings_reload_command_context.cc
110112
settings_reload_command_context.h
111113
single_state_steppable_context.cc

daemon/protocol/mcbp/session_validated_command_context.cc

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -40,41 +40,6 @@ cb::engine_errc SessionValidatedCommandContext::step() {
4040
return ret;
4141
}
4242

43-
static EngineParamCategory getParamCategory(Cookie& cookie) {
44-
const auto& req = cookie.getRequest();
45-
using cb::mcbp::request::SetParamPayload;
46-
const auto& payload = req.getCommandSpecifics<SetParamPayload>();
47-
switch (payload.getParamType()) {
48-
case SetParamPayload::Type::Flush:
49-
return EngineParamCategory::Flush;
50-
case SetParamPayload::Type::Checkpoint:
51-
return EngineParamCategory::Checkpoint;
52-
case SetParamPayload::Type::Dcp:
53-
return EngineParamCategory::Dcp;
54-
case SetParamPayload::Type::Vbucket:
55-
return EngineParamCategory::Vbucket;
56-
case SetParamPayload::Type::Replication:
57-
Expects(false && "mcbp_validator should reject this group");
58-
}
59-
throw std::invalid_argument("getParamCategory(): Invalid param provided: " +
60-
std::to_string(int(payload.getParamType())));
61-
}
62-
63-
SetParameterCommandContext::SetParameterCommandContext(Cookie& cookie)
64-
: SessionValidatedCommandContext(cookie),
65-
category(getParamCategory(cookie)) {
66-
}
67-
68-
cb::engine_errc SetParameterCommandContext::sessionLockedStep() {
69-
// We can't cache the key and value as a ewb would relocate them
70-
const auto& req = cookie.getRequest();
71-
return bucket_set_parameter(cookie,
72-
category,
73-
req.getKeyString(),
74-
req.getValueString(),
75-
req.getVBucket());
76-
}
77-
7843
cb::engine_errc CompactDatabaseCommandContext::sessionLockedStep() {
7944
return bucket_compact_database(cookie);
8045
}

daemon/protocol/mcbp/session_validated_command_context.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,6 @@ class SessionValidatedCommandContext : public SteppableCommandContext {
4343
const bool valid;
4444
};
4545

46-
/// Class to implement the SetParam command
47-
class SetParameterCommandContext : public SessionValidatedCommandContext {
48-
public:
49-
explicit SetParameterCommandContext(Cookie& cookie);
50-
51-
protected:
52-
cb::engine_errc sessionLockedStep() override;
53-
54-
private:
55-
const EngineParamCategory category;
56-
};
57-
5846
/// Class to implement the CompactDB command
5947
class CompactDatabaseCommandContext : public SessionValidatedCommandContext {
6048
public:
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright 2025-Present Couchbase, Inc.
3+
*
4+
* Use of this software is governed by the Business Source License included
5+
* in the file licenses/BSL-Couchbase.txt. As of the Change Date specified
6+
* in that file, in accordance with the Business Source License, use of this
7+
* software will be governed by the Apache License, Version 2.0, included in
8+
* the file licenses/APL2.txt.
9+
*/
10+
11+
#include "set_param_command_context.h"
12+
#include "engine_wrapper.h"
13+
#include <daemon/buckets.h>
14+
#include <daemon/concurrency_semaphores.h>
15+
#include <daemon/connection.h>
16+
#include <logger/logger.h>
17+
#include <memcached/engine.h>
18+
19+
EngineParamCategory SetParamCommandContext::getParamCategory(Cookie& cookie) {
20+
const auto& req = cookie.getRequest();
21+
using cb::mcbp::request::SetParamPayload;
22+
const auto& payload = req.getCommandSpecifics<SetParamPayload>();
23+
switch (payload.getParamType()) {
24+
case SetParamPayload::Type::Flush:
25+
return EngineParamCategory::Flush;
26+
case SetParamPayload::Type::Checkpoint:
27+
return EngineParamCategory::Checkpoint;
28+
case SetParamPayload::Type::Dcp:
29+
return EngineParamCategory::Dcp;
30+
case SetParamPayload::Type::Vbucket:
31+
return EngineParamCategory::Vbucket;
32+
case SetParamPayload::Type::Replication:
33+
Expects(false && "mcbp_validator should reject this group");
34+
}
35+
throw std::invalid_argument(
36+
"SetParamCommandContext::getParamCategory(): Invalid param "
37+
"provided: " +
38+
std::to_string(int(payload.getParamType())));
39+
}
40+
41+
SetParamCommandContext::SetParamCommandContext(Cookie& cookie)
42+
: SteppableCommandContext(cookie),
43+
category(getParamCategory(cookie)),
44+
key(cookie.getRequest().getKeyString()),
45+
value(cookie.getRequest().getValueString()),
46+
vbid(cookie.getRequest().getVBucket()) {
47+
}
48+
49+
cb::engine_errc SetParamCommandContext::step() {
50+
cb::engine_errc ret = cb::engine_errc::success;
51+
try {
52+
ret = bucket_set_parameter(cookie, category, key, value, vbid);
53+
} catch (const std::exception& e) {
54+
LOG_WARNING_CTX("SetParamCommandContext: ",
55+
{"conn_id", cookie.getConnectionId()},
56+
{"error", e.what()});
57+
}
58+
if (ret == cb::engine_errc::success) {
59+
cookie.sendResponse(ret);
60+
}
61+
return ret;
62+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright 2025-Present Couchbase, Inc.
3+
*
4+
* Use of this software is governed by the Business Source License included
5+
* in the file licenses/BSL-Couchbase.txt. As of the Change Date specified
6+
* in that file, in accordance with the Business Source License, use of this
7+
* software will be governed by the Apache License, Version 2.0, included in
8+
* the file licenses/APL2.txt.
9+
*/
10+
11+
#pragma once
12+
13+
#include "steppable_command_context.h"
14+
15+
/// Command context for executing SetParam requests
16+
class SetParamCommandContext : public SteppableCommandContext {
17+
public:
18+
explicit SetParamCommandContext(Cookie& cookie);
19+
20+
protected:
21+
static EngineParamCategory getParamCategory(Cookie& cookie);
22+
23+
cb::engine_errc step() override;
24+
const EngineParamCategory category;
25+
const std::string key;
26+
const std::string value;
27+
const Vbid vbid;
28+
};

tests/mcbp/mcbp_test_2.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,9 @@ TEST_P(SetParamValidatorTest, InvalidDatatype) {
405405
EXPECT_EQ(cb::mcbp::Status::Einval, validate());
406406
}
407407

408-
TEST_P(SetParamValidatorTest, Cas) {
408+
TEST_P(SetParamValidatorTest, InvalidCas) {
409409
req.setCas(0xff);
410-
EXPECT_EQ(cb::mcbp::Status::Success, validate());
410+
EXPECT_EQ(cb::mcbp::Status::Einval, validate());
411411
}
412412

413413
TEST_P(SetParamValidatorTest, InvalidKey) {

0 commit comments

Comments
 (0)