Skip to content

Commit a52ce0a

Browse files
jimwwalkerjimwwalker
committed
MB-29119: Replace revSeqno with a 48-bit counter
Prevent a value too large to be stored in couchstore from being placed into Item/StoredValue and also the _local document (via vbucket_state). Change-Id: I8613c0c51388e91612cde6216445a38c3351c190 Reviewed-on: http://review.couchbase.org/92485 Well-Formed: Build Bot <[email protected]> Reviewed-by: Dave Rigby <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent 43bc123 commit a52ce0a

File tree

10 files changed

+188
-34
lines changed

10 files changed

+188
-34
lines changed

‎engines/ep/src/ep_bucket.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ int EPBucket::flushVBucket(uint16_t vbid) {
308308
vbstate.maxCas = std::max(vbstate.maxCas, item->getCas());
309309
if (item->isDeleted()) {
310310
vbstate.maxDeletedSeqno =
311-
std::max(vbstate.maxDeletedSeqno,
311+
std::max(uint64_t(vbstate.maxDeletedSeqno),
312312
item->getRevSeqno());
313313
}
314314
++stats.flusher_todo;

‎engines/ep/src/item.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "config.h"
2121

2222
#include <memcached/engine.h>
23+
#include <platform/n_byte_integer.h>
2324
#include <stdio.h>
2425
#include <string.h>
2526
#include <utility>
@@ -113,7 +114,7 @@ class ItemMetaData {
113114
}
114115

115116
uint64_t cas;
116-
uint64_t revSeqno;
117+
cb::uint48_t revSeqno;
117118
uint32_t flags;
118119
time_t exptime;
119120
};

‎engines/ep/src/kvstore.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "logger.h"
2727

2828
#include <platform/histogram.h>
29+
#include <platform/n_byte_integer.h>
2930
#include <platform/processclock.h>
3031

3132
#include <cJSON.h>
@@ -172,7 +173,7 @@ struct vbucket_state {
172173

173174
vbucket_state_t state;
174175
uint64_t checkpointId;
175-
uint64_t maxDeletedSeqno;
176+
cb::uint48_t maxDeletedSeqno;
176177
int64_t highSeqno;
177178
uint64_t purgeSeqno;
178179
uint64_t lastSnapStart;

‎engines/ep/src/stored-value.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ StoredValue::StoredValue(const Item& itm,
3939
: value(itm.getValue()),
4040
chain_next_or_replacement(std::move(n)),
4141
cas(itm.getCas()),
42-
revSeqno(itm.getRevSeqno()),
4342
bySeqno(itm.getBySeqno()),
4443
lock_expiry_or_delete_time(0),
4544
exptime(itm.getExptime()),
4645
flags(itm.getFlags()),
46+
revSeqno(itm.getRevSeqno()),
4747
datatype(itm.getDataType()) {
4848
// Initialise bit fields
4949
setDeletedPriv(itm.isDeleted());
@@ -79,11 +79,11 @@ StoredValue::StoredValue(const StoredValue& other, UniquePtr n, EPStats& stats)
7979
: value(other.value),
8080
chain_next_or_replacement(std::move(n)),
8181
cas(other.cas),
82-
revSeqno(other.revSeqno),
8382
bySeqno(other.bySeqno),
8483
lock_expiry_or_delete_time(other.lock_expiry_or_delete_time),
8584
exptime(other.exptime),
8685
flags(other.flags),
86+
revSeqno(other.revSeqno),
8787
datatype(other.datatype) {
8888
setDirty(other.isDirty());
8989
setDeletedPriv(other.isDeleted());

‎engines/ep/src/stored-value.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "utility.h"
2626

2727
#include <memcached/3rd_party/folly/AtomicBitSet.h>
28+
#include <platform/n_byte_integer.h>
2829

2930
#include <boost/intrusive/list.hpp>
3031

@@ -722,12 +723,12 @@ class StoredValue {
722723
// only the newer version if so.
723724
UniquePtr chain_next_or_replacement; // 8 bytes
724725
uint64_t cas; //!< CAS identifier.
725-
uint64_t revSeqno; //!< Revision id sequence number
726726
int64_t bySeqno; //!< By sequence id number
727727
/// For alive items: GETL lock expiration. For deleted items: delete time.
728728
rel_time_t lock_expiry_or_delete_time;
729729
uint32_t exptime; //!< Expiration time of this item.
730730
uint32_t flags; // 4 bytes
731+
cb::uint48_t revSeqno; //!< Revision id sequence number
731732
protocol_binary_datatype_t datatype; // 1 byte
732733

733734
/**

‎engines/ep/tests/ep_test_apis.cc‎

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,9 @@ bool add_response_get_meta(const void *key, uint16_t keylen, const void *ext,
186186
memcpy(&last_meta.flags, ext_bytes + 4, 4);
187187
memcpy(&last_meta.exptime, ext_bytes + 8, 4);
188188
last_meta.exptime = ntohl(last_meta.exptime);
189-
memcpy(&last_meta.revSeqno, ext_bytes + 12, 8);
190-
last_meta.revSeqno = ntohll(last_meta.revSeqno);
189+
uint64_t revId = 0;
190+
memcpy(&revId, ext_bytes + 12, 8);
191+
last_meta.revSeqno = ntohll(revId);
191192
last_meta.cas = cas;
192193
if (extlen > 20) {
193194
memcpy(&meta_datatype, ext_bytes + 20, 1);
@@ -227,8 +228,9 @@ bool add_response_ret_meta(const void *key, uint16_t keylen, const void *ext,
227228
memcpy(&last_meta.flags, ext_bytes, 4);
228229
memcpy(&last_meta.exptime, ext_bytes + 4, 4);
229230
last_meta.exptime = ntohl(last_meta.exptime);
230-
memcpy(&last_meta.revSeqno, ext_bytes + 8, 8);
231-
last_meta.revSeqno = ntohll(last_meta.revSeqno);
231+
uint64_t revId = 0;
232+
memcpy(&revId, ext_bytes + 8, 8);
233+
last_meta.revSeqno = ntohll(revId);
232234
last_meta.cas = cas;
233235
}
234236
return add_response(key, keylen, ext, extlen, body, bodylen, datatype,
@@ -302,16 +304,31 @@ void encodeExt(char *buffer, uint32_t val) {
302304
memcpy(buffer, (char*)&val, sizeof(val));
303305
}
304306

305-
void encodeWithMetaExt(char *buffer, ItemMetaData *meta) {
307+
void encodeWithMetaExt(char* buffer,
308+
uint64_t cas,
309+
uint64_t revSeqno,
310+
uint32_t flags,
311+
uint32_t exp) {
312+
memcpy(buffer, (char*)&flags, sizeof(flags));
313+
memcpy(buffer + 4, (char*)&exp, sizeof(exp));
314+
memcpy(buffer + 8, (char*)&revSeqno, sizeof(revSeqno));
315+
memcpy(buffer + 16, (char*)&cas, sizeof(cas));
316+
}
317+
318+
void encodeWithMetaExt(char* buffer, RawItemMetaData* meta) {
306319
uint32_t flags = meta->flags;
307320
uint32_t exp = htonl(meta->exptime);
308321
uint64_t seqno = htonll(meta->revSeqno);
309322
uint64_t cas = htonll(meta->cas);
323+
encodeWithMetaExt(buffer, cas, seqno, flags, exp);
324+
}
310325

311-
memcpy(buffer, (char*)&flags, sizeof(flags));
312-
memcpy(buffer + 4, (char*)&exp, sizeof(exp));
313-
memcpy(buffer + 8, (char*)&seqno, sizeof(seqno));
314-
memcpy(buffer + 16, (char*)&cas, sizeof(cas));
326+
void encodeWithMetaExt(char* buffer, ItemMetaData* meta) {
327+
uint32_t flags = meta->flags;
328+
uint32_t exp = htonl(meta->exptime);
329+
uint64_t seqno = htonll(meta->revSeqno);
330+
uint64_t cas = htonll(meta->cas);
331+
encodeWithMetaExt(buffer, cas, seqno, flags, exp);
315332
}
316333

317334
protocol_binary_request_header* createPacket(uint8_t opcode,
@@ -422,6 +439,36 @@ void del_with_meta(ENGINE_HANDLE* h,
422439
const std::vector<char>& nmeta,
423440
protocol_binary_datatype_t datatype,
424441
const std::vector<char>& value) {
442+
RawItemMetaData meta{itemMeta->cas,
443+
itemMeta->revSeqno,
444+
itemMeta->flags,
445+
itemMeta->exptime};
446+
del_with_meta(h,
447+
h1,
448+
key,
449+
keylen,
450+
vb,
451+
&meta,
452+
cas_for_delete,
453+
options,
454+
cookie,
455+
nmeta,
456+
datatype,
457+
value);
458+
}
459+
460+
void del_with_meta(ENGINE_HANDLE* h,
461+
ENGINE_HANDLE_V1* h1,
462+
const char* key,
463+
const size_t keylen,
464+
const uint32_t vb,
465+
RawItemMetaData* itemMeta,
466+
uint64_t cas_for_delete,
467+
uint32_t options,
468+
const void* cookie,
469+
const std::vector<char>& nmeta,
470+
protocol_binary_datatype_t datatype,
471+
const std::vector<char>& value) {
425472
int blen = 24;
426473
std::unique_ptr<char[]> ext(new char[30]);
427474
std::unique_ptr<ExtendedMetaData> emd;

‎engines/ep/tests/ep_test_apis.h‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,28 @@ class WaitTimeAccumulator
152152
useconds_t totalSleepTime;
153153
};
154154

155+
/**
156+
* Raw meta-data allowing 64-bit rev-seqno
157+
*/
158+
class RawItemMetaData {
159+
public:
160+
RawItemMetaData()
161+
: cas(0), revSeqno(DEFAULT_REV_SEQ_NUM), flags(0), exptime(0) {
162+
}
163+
164+
RawItemMetaData(uint64_t c, uint64_t s, uint32_t f, time_t e)
165+
: cas(c),
166+
revSeqno(s == 0 ? DEFAULT_REV_SEQ_NUM : s),
167+
flags(f),
168+
exptime(e) {
169+
}
170+
171+
uint64_t cas;
172+
uint64_t revSeqno;
173+
uint32_t flags;
174+
time_t exptime;
175+
};
176+
155177
void decayingSleep(useconds_t *sleepTime);
156178

157179

@@ -464,6 +486,20 @@ void del_with_meta(ENGINE_HANDLE* h,
464486
protocol_binary_datatype_t datatype = 0,
465487
const std::vector<char>& value = {} /*optional value*/);
466488

489+
// This version takes a RawItemMetaData allowing for 64-bit rev-seqno tests
490+
void del_with_meta(ENGINE_HANDLE* h,
491+
ENGINE_HANDLE_V1* h1,
492+
const char* key,
493+
const size_t keylen,
494+
const uint32_t vb,
495+
RawItemMetaData* itemMeta,
496+
uint64_t cas_for_delete = 0,
497+
uint32_t options = 0,
498+
const void* cookie = nullptr,
499+
const std::vector<char>& nmeta = {},
500+
protocol_binary_datatype_t datatype = 0,
501+
const std::vector<char>& value = {} /*optional value*/);
502+
467503
void return_meta(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1, const char *key,
468504
const size_t keylen, const char *val, const size_t vallen,
469505
const uint32_t vb, const uint64_t cas, const uint32_t flags,

‎engines/ep/tests/ep_testsuite.cc‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,8 @@ static enum test_result test_expiry_with_xattr(ENGINE_HANDLE* h,
855855
get_meta(h, h1, "test_expiry", false, GetMetaVersion::V1, cookie),
856856
"Get meta command failed");
857857

858-
checkeq(last_meta.revSeqno, prev_revseqno + 1,
858+
checkeq(uint64_t(last_meta.revSeqno),
859+
prev_revseqno + 1,
859860
"rev seqno must have incremented by 1");
860861

861862
/* Retrieve the item info and create a new blob out of the data */
@@ -4165,7 +4166,7 @@ static enum test_result test_revid(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1)
41654166

41664167
checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS, last_status.load(),
41674168
"Expected success");
4168-
checkeq(ii, last_meta.revSeqno, "Unexpected sequence number");
4169+
checkeq(ii, uint64_t(last_meta.revSeqno), "Unexpected sequence number");
41694170
}
41704171

41714172
return SUCCESS;
@@ -4993,7 +4994,7 @@ static enum test_result test_set_ret_meta(ENGINE_HANDLE *h,
49934994
check(last_meta.flags == 0, "Invalid result for flags");
49944995
check(last_meta.exptime == 0, "Invalid result for expiration");
49954996
check(last_meta.cas != 0, "Invalid result for cas");
4996-
check(last_meta.revSeqno == 1, "Invalid result for seqno");
4997+
check(last_meta.revSeqno == 1ull, "Invalid result for seqno");
49974998

49984999
// Check that set with correct cas succeeds
49995000
set_ret_meta(h, h1, "key", 3, "value", 5, 0, last_meta.cas, 10, 1735689600);
@@ -5005,7 +5006,7 @@ static enum test_result test_set_ret_meta(ENGINE_HANDLE *h,
50055006
check(last_meta.flags == 10, "Invalid result for flags");
50065007
check(last_meta.exptime == 1735689600, "Invalid result for expiration");
50075008
check(last_meta.cas != 0, "Invalid result for cas");
5008-
check(last_meta.revSeqno == 2, "Invalid result for seqno");
5009+
check(last_meta.revSeqno == 2ull, "Invalid result for seqno");
50095010

50105011
// Check that updating an item with no cas succeeds
50115012
set_ret_meta(h, h1, "key", 3, "value", 5, 0, 0, 5, 0);
@@ -5017,7 +5018,7 @@ static enum test_result test_set_ret_meta(ENGINE_HANDLE *h,
50175018
check(last_meta.flags == 5, "Invalid result for flags");
50185019
check(last_meta.exptime == 0, "Invalid result for expiration");
50195020
check(last_meta.cas != 0, "Invalid result for cas");
5020-
check(last_meta.revSeqno == 3, "Invalid result for seqno");
5021+
check(last_meta.revSeqno == 3ull, "Invalid result for seqno");
50215022

50225023
// Check that updating an item with the wrong cas fails
50235024
set_ret_meta(h, h1, "key", 3, "value", 5, 0, last_meta.cas + 1, 5, 0);
@@ -5091,7 +5092,7 @@ static enum test_result test_add_ret_meta(ENGINE_HANDLE *h,
50915092
check(last_meta.flags == 0, "Invalid result for flags");
50925093
check(last_meta.exptime == 0, "Invalid result for expiration");
50935094
check(last_meta.cas != 0, "Invalid result for cas");
5094-
check(last_meta.revSeqno == 1, "Invalid result for seqno");
5095+
check(last_meta.revSeqno == 1ull, "Invalid result for seqno");
50955096

50965097
// Check that re-adding a key fails
50975098
add_ret_meta(h, h1, "key", 3, "value", 5, 0, 0, 0, 0);
@@ -5108,7 +5109,7 @@ static enum test_result test_add_ret_meta(ENGINE_HANDLE *h,
51085109
check(last_meta.flags == 10, "Invalid result for flags");
51095110
check(last_meta.exptime == 1735689600, "Invalid result for expiration");
51105111
check(last_meta.cas != 0, "Invalid result for cas");
5111-
check(last_meta.revSeqno == 1, "Invalid result for seqno");
5112+
check(last_meta.revSeqno == 1ull, "Invalid result for seqno");
51125113

51135114
return SUCCESS;
51145115
}
@@ -5178,7 +5179,7 @@ static enum test_result test_del_ret_meta(ENGINE_HANDLE *h,
51785179
check(last_meta.flags == 0, "Invalid result for flags");
51795180
check(last_meta.exptime == 0, "Invalid result for expiration");
51805181
check(last_meta.cas != 0, "Invalid result for cas");
5181-
check(last_meta.revSeqno == 1, "Invalid result for seqno");
5182+
check(last_meta.revSeqno == 1ull, "Invalid result for seqno");
51825183

51835184
del_ret_meta(h, h1, "key", 3, 0, 0);
51845185
checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS, last_status.load(),
@@ -5189,7 +5190,7 @@ static enum test_result test_del_ret_meta(ENGINE_HANDLE *h,
51895190
check(last_meta.flags == 0, "Invalid result for flags");
51905191
check(last_meta.exptime == 0, "Invalid result for expiration");
51915192
check(last_meta.cas != 0, "Invalid result for cas");
5192-
check(last_meta.revSeqno == 2, "Invalid result for seqno");
5193+
check(last_meta.revSeqno == 2ull, "Invalid result for seqno");
51935194

51945195
// Check that deleting a key with a cas succeeds.
51955196
add_ret_meta(h, h1, "key", 3, "value", 5, 0, 0, 10, 1735689600);
@@ -5199,7 +5200,7 @@ static enum test_result test_del_ret_meta(ENGINE_HANDLE *h,
51995200
check(last_meta.flags == 10, "Invalid result for flags");
52005201
check(last_meta.exptime == 1735689600, "Invalid result for expiration");
52015202
check(last_meta.cas != 0, "Invalid result for cas");
5202-
check(last_meta.revSeqno == 3, "Invalid result for seqno");
5203+
check(last_meta.revSeqno == 3ull, "Invalid result for seqno");
52035204

52045205
del_ret_meta(h, h1, "key", 3, 0, last_meta.cas);
52055206
checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS, last_status.load(),
@@ -5210,7 +5211,7 @@ static enum test_result test_del_ret_meta(ENGINE_HANDLE *h,
52105211
check(last_meta.flags == 10, "Invalid result for flags");
52115212
check(last_meta.exptime == 1735689600, "Invalid result for expiration");
52125213
check(last_meta.cas != 0, "Invalid result for cas");
5213-
check(last_meta.revSeqno == 4, "Invalid result for seqno");
5214+
check(last_meta.revSeqno == 4ull, "Invalid result for seqno");
52145215

52155216
// Check that deleting a key with the wrong cas fails
52165217
add_ret_meta(h, h1, "key", 3, "value", 5, 0, 0, 0, 0);
@@ -5220,7 +5221,7 @@ static enum test_result test_del_ret_meta(ENGINE_HANDLE *h,
52205221
check(last_meta.flags == 0, "Invalid result for flags");
52215222
check(last_meta.exptime == 0, "Invalid result for expiration");
52225223
check(last_meta.cas != 0, "Invalid result for cas");
5223-
check(last_meta.revSeqno == 5, "Invalid result for seqno");
5224+
check(last_meta.revSeqno == 5ull, "Invalid result for seqno");
52245225

52255226
del_ret_meta(h, h1, "key", 3, 0, last_meta.cas + 1);
52265227
checkeq(PROTOCOL_BINARY_RESPONSE_KEY_EEXISTS, last_status.load(),

‎engines/ep/tests/ep_testsuite_basic.cc‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,7 +1289,8 @@ static enum test_result test_delete_with_value_cas(ENGINE_HANDLE *h,
12891289
checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
12901290
last_status.load(), "Get meta failed");
12911291

1292-
checkeq(last_meta.revSeqno, curr_revseqno + 1,
1292+
checkeq(uint64_t(last_meta.revSeqno),
1293+
curr_revseqno + 1,
12931294
"rev seqno should have incremented");
12941295

12951296
item *i = nullptr;
@@ -1321,7 +1322,8 @@ static enum test_result test_delete_with_value_cas(ENGINE_HANDLE *h,
13211322
checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
13221323
last_status.load(), "Get meta failed");
13231324

1324-
checkeq(last_meta.revSeqno, curr_revseqno + 1,
1325+
checkeq(uint64_t(last_meta.revSeqno),
1326+
curr_revseqno + 1,
13251327
"rev seqno should have incremented");
13261328

13271329
curr_revseqno = last_meta.revSeqno;
@@ -1345,7 +1347,8 @@ static enum test_result test_delete_with_value_cas(ENGINE_HANDLE *h,
13451347
checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
13461348
last_status.load(), "Get meta failed");
13471349

1348-
checkeq(last_meta.revSeqno, curr_revseqno + 1,
1350+
checkeq(uint64_t(last_meta.revSeqno),
1351+
curr_revseqno + 1,
13491352
"rev seqno should have incremented");
13501353

13511354
curr_revseqno = last_meta.revSeqno;
@@ -1373,7 +1376,8 @@ static enum test_result test_delete_with_value_cas(ENGINE_HANDLE *h,
13731376
checkeq(PROTOCOL_BINARY_RESPONSE_SUCCESS,
13741377
last_status.load(), "Get meta failed");
13751378

1376-
checkeq(last_meta.revSeqno, curr_revseqno + 1,
1379+
checkeq(uint64_t(last_meta.revSeqno),
1380+
curr_revseqno + 1,
13771381
"rev seqno should have incremented");
13781382

13791383
check(h1->get_item_info(h, nullptr, ret.second.get(), &info),

0 commit comments

Comments
 (0)