From 747e95b0544f80a0488351955ac7ee643b0f1a1c Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Thu, 30 Jul 2026 13:12:53 -0400 Subject: [PATCH 01/13] bdb: add odh2 on-disk header codec (insert/update timestamps, 32-bit length) Introduce a second on-disk record header format ("odh2") alongside the existing 7-byte odh1 header. odh2 is a strict superset of odh1: the flags, csc2vers, and updateid bytes are encoded identically, so odh1 readers and writers are unaffected and the two formats coexist in the same table. odh2 (16 bytes) is discriminated by ODH2_FLAG (bit 7 of the flags byte), which odh1 never sets. It replaces odh1's packed 28-bit length with a clean 32-bit length field and appends two 32-bit unsigned timestamps: insert_secs and update_secs (seconds since the 1970 epoch, stored unsigned to survive the 2038 signed-time_t rollover). The wider length lifts the odh1 256MB ceiling. This commit adds only the codec and plumbing primitives: - struct odh gains insert_secs / update_secs - ODH2_SIZE, ODH2_FLAG, odh_size_from_flags(); ODH_SIZE_RESERVE bumped to the max header size so buffers fit either format - write_odh / read_odh branch on ODH2_FLAG; unpack peeks the flags byte to learn the real header size before validating - IPU paths (bdb_update_updateid, bdb_cposition) read into ODH2_SIZE buffers and rewrite exactly odh_size_from_flags() bytes - poke_update_secs() helper for stamping update time in place Nothing sets ODH2_FLAG yet, so no odh2 record is produced; this is the codec baseline. Also rewrites the odh.c header diagram to document both layouts and removes the stale note-to-self. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/bdb_int.h | 50 ++++++-- bdb/odh.c | 330 ++++++++++++++++++++++++++++---------------------- 2 files changed, 220 insertions(+), 160 deletions(-) diff --git a/bdb/bdb_int.h b/bdb/bdb_int.h index 8f3d0b1714..15418d345a 100644 --- a/bdb/bdb_int.h +++ b/bdb/bdb_int.h @@ -62,19 +62,36 @@ enum { ODH_UPDATEID_BITS = 12, ODH_LENGTH_BITS = 28, - ODH_SIZE = 7, /* We may extend for larger headers in the future, - but the minimum size shall always be 7 bytes. */ - - ODH_SIZE_RESERVE = 7, /* Callers wishing to provide a buffer into which - a record will be packed should allow this many - bytes on top of the record size for the ODH. - Right now this is the same as ODH_SIZE - one - day it may be the max possible ODH size if we - start adding fields. */ - - ODH_FLAG_COMPR_MASK = 0x7 + ODH_SIZE = 7, /* Size of the original (odh1) on-disk header. This is the + minimum ODH size and shall always be 7 bytes. */ + + ODH2_SIZE = 16, /* Size of the version-2 (odh2) on-disk header. odh2 is a + strict superset of odh1: flags/csc2vers/updateid occupy + the same bytes, length becomes a clean 32-bit field, and + 32-bit insert/update timestamps are appended. odh2 + records are flagged by ODH2_FLAG in the flags byte. */ + + ODH_SIZE_RESERVE = ODH2_SIZE, /* Callers wishing to provide a buffer into + which a record will be packed should allow this + many bytes on top of the record size for the ODH. + This must be the MAXIMUM possible header size so a + buffer fits either an odh1 or an odh2 header. */ + + ODH_FLAG_COMPR_MASK = 0x7, /* flags bits 0-2: compression algorithm */ + + ODH2_FLAG = 0x80 /* flags bit 7: set on odh2 records. odh1 records only + ever set the compression bits (0-2), so this bit is an + unambiguous odh1/odh2 discriminator. bit 3 is reserved + for future compression-mask growth. */ }; +/* Actual on-disk header size implied by a record's flags byte. Only valid + * when ondisk_header is enabled for the table. */ +static inline int odh_size_from_flags(uint8_t flags) +{ + return (flags & ODH2_FLAG) ? ODH2_SIZE : ODH_SIZE; +} + /* snapisol log ops */ typedef enum log_ops { LOG_APPLY = 0, LOG_PRESCAN = 1, LOG_BACKFILL = 2 } log_ops_t; @@ -82,13 +99,19 @@ typedef enum log_ops { LOG_APPLY = 0, LOG_PRESCAN = 1, LOG_BACKFILL = 2 } log_op * representation but a convenient format for passing the header around in * our code. */ struct odh { - uint32_t length; /* actually only 28 bits of this can be used leading to - a max value of (1< odh1, 1 => odh2 + * bits 6-4 reserved + * bit 3 reserved (future growth of the compression mask) + * bits 2-0 compression algorithm (ODH_FLAG_COMPR_MASK) + * + * odh1 -- 7 bytes (ODH_SIZE). length is packed into 28 bits, giving a maximum + * record/blob size of (1<<28)-1 (~256MB): + * + * byte 0 flags (ODH2_FLAG clear) + * byte 1 csc2vers + * byte 2 updateid bits [11:4] + * byte 3 updateid bits [3:0] (high nibble) | length bits [27:24] (low) + * byte 4 length bits [23:16] + * byte 5 length bits [15:8] + * byte 6 length bits [7:0] + * + * odh2 -- 16 bytes (ODH2_SIZE). length becomes a clean 32-bit field and two + * 32-bit timestamps are appended after it: + * + * byte 0 flags (ODH2_FLAG set) + * byte 1 csc2vers + * byte 2 updateid bits [11:4] + * byte 3 updateid bits [3:0] (high nibble) | reserved (low nibble, 0) + * bytes 4-7 length (32-bit big-endian) + * bytes 8-11 insert_secs (32-bit big-endian) -- record's insert time + * bytes12-15 update_secs (32-bit big-endian) -- record's last-update time + * + * The timestamps are seconds since the 1970 epoch, stored *unsigned* so they + * remain valid past the 2038 signed-time_t rollover (goals #1 and #2). The + * 32-bit length lifts the odh1 256MB ceiling (goal #3): the field can hold up + * to 4GB-1, and the intent is to cap writes at INT_MAX (~2GB) so lengths stay + * safe in the signed-int code paths above bdb. NB: as of this writing that cap + * is not yet enforced -- the existing MAXBLOBLENGTH check ((1<<28)-1) in + * db/toblock.c still gates blob writes, and nothing sets ODH2_FLAG yet, so no + * odh2 record is actually produced. The codec below is ready for both. */ /* Return 1 if ip-updates are enabled. Does not care about schema-change */ @@ -196,32 +181,69 @@ void poke_updateid(void *buf, int updid) /**out = ((updid << 4) & 0xf0) | ((len >> 24) & 0x0f);*/ } +/* Subset of write_odh which pokes only the update timestamp. odh2 only - the + * caller MUST have verified the buffer holds an odh2 record. update_secs lives + * at bytes 12-15 (big-endian). */ +void poke_update_secs(void *buf, uint32_t secs) +{ + uint8_t *out = buf; + out[12] = (secs >> 24) & 0xff; + out[13] = (secs >> 16) & 0xff; + out[14] = (secs >> 8) & 0xff; + out[15] = (secs & 0xff); +} + /* You MUST range check updateid and length before calling this or it can get - * ugly if bits are set that shouldn't be set */ + * ugly if bits are set that shouldn't be set. + * + * odh2 is a strict superset of odh1: bytes 0-2 and the high nibble of byte 3 + * (flags, csc2vers, updateid) are encoded identically. The layouts diverge + * only for length (odh1 packs the top nibble into byte 3 and the low 24 bits + * into bytes 4-6; odh2 uses a clean 32-bit field at bytes 4-7) and for the two + * appended 32-bit timestamps. The ODH2_FLAG bit in flags selects the layout. + */ static void write_odh(void *buf, const struct odh *odh, uint8_t flags) { uint32_t len = odh->length; uint16_t updid = odh->updateid; uint8_t *out = buf; - /* byte 1: flags */ - *out = flags; - out++; - /* byte 2: csc2 version */ - *out = odh->csc2vers; - out++; - /* byte 3: high 8 bits of updateid */ - *out = (updid >> 4); - out++; - /* byte 4: low 4 bits of updateid and then highest 4 bits of length */ - *out = ((updid << 4) & 0xf0) | ((len >> 24) & 0x0f); - out++; - /* bytes 5-7: remaining bits of length */ - *out = ((len >> 16) & 0xff); - out++; - *out = ((len >> 8) & 0xff); - out++; - *out = (len & 0xff); + /* byte 0: flags (shared) */ + out[0] = flags; + /* byte 1: csc2 version (shared) */ + out[1] = odh->csc2vers; + /* byte 2: high 8 bits of updateid (shared) */ + out[2] = (updid >> 4); + + if (flags & ODH2_FLAG) { + uint32_t ins = odh->insert_secs; + uint32_t upd = odh->update_secs; + /* byte 3: low 4 bits of updateid; low nibble spare (0) */ + out[3] = ((updid << 4) & 0xf0); + /* bytes 4-7: full 32-bit length */ + out[4] = (len >> 24) & 0xff; + out[5] = (len >> 16) & 0xff; + out[6] = (len >> 8) & 0xff; + out[7] = (len & 0xff); + /* bytes 8-11: insert_secs */ + out[8] = (ins >> 24) & 0xff; + out[9] = (ins >> 16) & 0xff; + out[10] = (ins >> 8) & 0xff; + out[11] = (ins & 0xff); + /* bytes 12-15: update_secs */ + out[12] = (upd >> 24) & 0xff; + out[13] = (upd >> 16) & 0xff; + out[14] = (upd >> 8) & 0xff; + out[15] = (upd & 0xff); + return; + } + + /* odh1: byte 3 low nibble holds the highest 4 bits of length */ + out[3] = ((updid << 4) & 0xf0) | ((len >> 24) & 0x0f); + /* bytes 4-6: remaining bits of length */ + out[4] = ((len >> 16) & 0xff); + out[5] = ((len >> 8) & 0xff); + out[6] = (len & 0xff); } static void read_odh(const void *buf, struct odh *odh) @@ -230,29 +252,32 @@ static void read_odh(const void *buf, struct odh *odh) uint32_t len; uint16_t updid; - /* byte 1: flags */ - odh->flags = *in; - in++; - /* byte 2: csc2 version */ - odh->csc2vers = *in; - in++; - /* byte 3: high 8 bits of updateid */ - updid = (*in << 4); - in++; - /* byte 4: low 4 bits of updateid and then highest 4 bits of length */ - updid |= (*in >> 4); + /* byte 0: flags (shared) */ + odh->flags = in[0]; + /* byte 1: csc2 version (shared) */ + odh->csc2vers = in[1]; + /* bytes 2-3: high 8 bits then low 4 bits of updateid (shared) */ + updid = ((uint16_t)in[2] << 4); + updid |= (in[3] >> 4); odh->updateid = updid; - /* bytes 5-7: remaining bits of length */ - len = ((*in & 0x0f) << 24); - in++; - len |= ((*in & 0xff) << 16); - in++; - len |= ((*in & 0xff) << 8); - in++; - len |= (*in & 0xff); + if (in[0] & ODH2_FLAG) { + /* odh2: clean 32-bit length + two 32-bit timestamps */ + odh->length = ((uint32_t)in[4] << 24) | ((uint32_t)in[5] << 16) | ((uint32_t)in[6] << 8) | in[7]; + odh->insert_secs = ((uint32_t)in[8] << 24) | ((uint32_t)in[9] << 16) | ((uint32_t)in[10] << 8) | in[11]; + odh->update_secs = ((uint32_t)in[12] << 24) | ((uint32_t)in[13] << 16) | ((uint32_t)in[14] << 8) | in[15]; + return; + } + /* odh1: byte 3 low nibble holds the highest 4 bits of length; bytes 4-6 + * hold the remaining bits. No timestamps. */ + len = ((uint32_t)(in[3] & 0x0f) << 24); + len |= ((uint32_t)in[4] << 16); + len |= ((uint32_t)in[5] << 8); + len |= in[6]; odh->length = len; + odh->insert_secs = 0; + odh->update_secs = 0; } void init_odh(bdb_state_type *bdb_state, struct odh *odh, void *rec, @@ -265,6 +290,8 @@ void init_odh(bdb_state_type *bdb_state, struct odh *odh, void *rec, else odh->csc2vers = 0; odh->flags = 0; + odh->insert_secs = 0; + odh->update_secs = 0; odh->recptr = rec; if (is_blob) { odh->flags |= (bdb_state->compress_blobs & ODH_FLAG_COMPR_MASK); @@ -307,6 +334,10 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, void *mallocmem = NULL; uint8_t flags = odh->flags; int alg; + /* Header size implied by the flags we will actually write. The + * compression-mask clear below never touches ODH2_FLAG, so this is + * stable for the whole function. */ + const int hdrsz = odh_size_from_flags(flags); /* We will need a buffer to do this in. Eventually we'll refactor all * the @@ -314,7 +345,7 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, * record we'll have ODH_SIZE_RESERVE spare before the record data so we * can avoid an allocate and copy here.. but for now we have to copy. */ if (to) { - if (tolen < odh->length + ODH_SIZE) { + if (tolen < (size_t)odh->length + hdrsz) { logmsg(LOGMSG_ERROR, "%s:ERROR: to buffer too small at %u bytes for " "%u byte record\n", __func__, (unsigned)tolen, (unsigned)odh->length); @@ -322,16 +353,14 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, } mallocmem = NULL; } else { - if ((odh->length + ODH_SIZE) > bdb_state->bmaszthresh) - mallocmem = - comdb2_bmalloc(bdb_state->bma, odh->length + ODH_SIZE); + if (((size_t)odh->length + hdrsz) > bdb_state->bmaszthresh) + mallocmem = comdb2_bmalloc(bdb_state->bma, (size_t)odh->length + hdrsz); else - mallocmem = malloc(odh->length + ODH_SIZE); + mallocmem = malloc((size_t)odh->length + hdrsz); if (!mallocmem) { rc = errno; - logmsg(LOGMSG_ERROR, "%s: out of memory %u\n", __func__, - (unsigned)odh->length + ODH_SIZE); + logmsg(LOGMSG_ERROR, "%s: out of memory %u\n", __func__, (unsigned)odh->length + hdrsz); return rc; } to = mallocmem; @@ -349,8 +378,7 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, * then I'm not interested. */ uLongf destLen = odh->length; - rc = compress2(((Bytef *)to) + ODH_SIZE, &destLen, - (const Bytef *)odh->recptr, odh->length, + rc = compress2(((Bytef *)to) + hdrsz, &destLen, (const Bytef *)odh->recptr, odh->length, bdb_state->attr->zlib_level); switch (rc) { default: @@ -375,15 +403,14 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, logmsg(LOGMSG_USER, "%s compressed %u bytes -> %u\n", bdb_state->name, (unsigned)odh->length, (unsigned)destLen); } - *recsize = destLen + ODH_SIZE; + *recsize = destLen + hdrsz; break; } break; } case BDB_COMPRESS_RLE8: - rc = rle8_compress(odh->recptr, odh->length, - ((Bytef *)to) + ODH_SIZE, odh->length); + rc = rle8_compress(odh->recptr, odh->length, ((Bytef *)to) + hdrsz, odh->length); if (rc < 0) { alg = BDB_COMPRESS_NONE; if (bdb_state->attr->ztrace) { @@ -396,38 +423,34 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, bdb_state->name, (unsigned)odh->length, (unsigned)rc); } - *recsize = rc + ODH_SIZE; + *recsize = rc + hdrsz; } break; case BDB_COMPRESS_CRLE: { - Comdb2RLE rle = {.in = odh->recptr, - .insz = odh->length, - .out = (uint8_t *)to + ODH_SIZE, - .outsz = odh->length - 1}; + Comdb2RLE rle = { + .in = odh->recptr, .insz = odh->length, .out = (uint8_t *)to + hdrsz, .outsz = odh->length - 1}; uint16_t *fld_hints = pd_index != -1 ? bdb_state->fld_hints_pd[pd_index] : bdb_state->fld_hints; if (compressComdb2RLE_hints(&rle, fld_hints) == 0) - *recsize = rle.outsz + ODH_SIZE; + *recsize = rle.outsz + hdrsz; else alg = BDB_COMPRESS_NONE; break; } case BDB_COMPRESS_LZ4: - if ((rc = LZ4_compress_default( - odh->recptr, (char *)to + ODH_SIZE, odh->length, - odh->length - 1)) == 0) { + if ((rc = LZ4_compress_default(odh->recptr, (char *)to + hdrsz, odh->length, odh->length - 1)) == 0) { alg = BDB_COMPRESS_NONE; } else { - *recsize = rc + ODH_SIZE; + *recsize = rc + hdrsz; } break; } if (alg == BDB_COMPRESS_NONE) { /* No compression, or compression was no good. */ - memcpy(((char *)to) + ODH_SIZE, odh->recptr, odh->length); - *recsize = odh->length + ODH_SIZE; + memcpy(((char *)to) + hdrsz, odh->recptr, odh->length); + *recsize = odh->length + hdrsz; flags &= ~ODH_FLAG_COMPR_MASK; } write_odh(to, odh, flags); @@ -494,6 +517,7 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from if (bdb_state->ondisk_header || force_odh) { int alg; + int hdrsz; if (fromlen < ODH_SIZE) { logmsg(LOGMSG_ERROR, "%s:ERROR: data size %u too small for ODH\n", @@ -501,6 +525,14 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from return DB_ODH_CORRUPT; } + /* Peek at the flags byte to learn the actual header size, then make + * sure the record is large enough for a full (possibly odh2) header. */ + hdrsz = odh_size_from_flags(*(const uint8_t *)from); + if (fromlen < hdrsz) { + logmsg(LOGMSG_ERROR, "%s:ERROR: data size %u too small for odh2\n", __func__, (unsigned)fromlen); + return DB_ODH_CORRUPT; + } + read_odh(from, odh); if ((verify_updateid) && (updateid >= 0) && @@ -538,11 +570,11 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from do_uncompress = 0; } else { if (fn_malloc != NULL) - to = fn_malloc((int)(odh->length + ver_bytes)); - else if ((odh->length + ver_bytes) > bdb_state->bmaszthresh) - to = comdb2_bmalloc(bdb_state->bma, odh->length + ver_bytes); + to = fn_malloc((size_t)odh->length + ver_bytes); + else if (((size_t)odh->length + ver_bytes) > bdb_state->bmaszthresh) + to = comdb2_bmalloc(bdb_state->bma, (size_t)odh->length + ver_bytes); else - to = malloc(odh->length + ver_bytes); + to = malloc((size_t)odh->length + ver_bytes); if (!to) { rc = errno; @@ -558,14 +590,11 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from if (do_uncompress == 0) { /* Do nothing */ } else if (alg == BDB_COMPRESS_ZLIB) { - rc = uncompress(to, &destLen, ((Bytef *)from) + ODH_SIZE, - fromlen - ODH_SIZE); + rc = uncompress(to, &destLen, ((Bytef *)from) + hdrsz, fromlen - hdrsz); if (rc != Z_OK) { - logmsg(LOGMSG_ERROR, "%s:uncompress gave %d %s %u->%u\n", - __func__, rc, zError(rc), - (unsigned)fromlen - ODH_SIZE, - (unsigned)odh->length); + logmsg(LOGMSG_ERROR, "%s:uncompress gave %d %s %u->%u\n", __func__, rc, zError(rc), + (unsigned)fromlen - hdrsz, (unsigned)odh->length); goto err; } @@ -576,19 +605,15 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from goto err; } } else if (alg == BDB_COMPRESS_RLE8) { - rc = rle8_decompress(((const char *)from) + ODH_SIZE, - fromlen - ODH_SIZE, to, odh->length); + rc = rle8_decompress(((const char *)from) + hdrsz, fromlen - hdrsz, to, odh->length); if (rc != odh->length) { - logmsg(LOGMSG_ERROR, - "%s:ERROR rle_decompress rc %d expected %u\n", - __func__, rc, (unsigned)odh->length); + logmsg(LOGMSG_ERROR, "%s:ERROR rle_decompress rc %d expected %u\n", __func__, rc, + (unsigned)odh->length); goto err; } } else if (alg == BDB_COMPRESS_CRLE) { - Comdb2RLE rle = {.in = (uint8_t *)from + ODH_SIZE, - .insz = fromlen - ODH_SIZE, - .out = to, - .outsz = odh->length}; + Comdb2RLE rle = { + .in = (uint8_t *)from + hdrsz, .insz = fromlen - hdrsz, .out = to, .outsz = odh->length}; rc = decompressComdb2RLE(&rle); if (rc || rle.outsz != odh->length) { logmsg(LOGMSG_ERROR, @@ -598,8 +623,7 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from goto err; } } else if (alg == BDB_COMPRESS_LZ4) { - rc = LZ4_decompress_safe((char *)from + ODH_SIZE, to, - (fromlen - ODH_SIZE), odh->length); + rc = LZ4_decompress_safe((char *)from + hdrsz, to, (fromlen - hdrsz), odh->length); if (rc != odh->length) { goto err; } @@ -613,12 +637,12 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from } else { /* No compression, just point to where the record data lives in * this record. */ - if (odh->length != fromlen - ODH_SIZE) { + if (odh->length != fromlen - hdrsz) { logmsg(LOGMSG_ERROR, "%s:ERROR: odh->length=%u, fromlen=%u\n", __func__, (unsigned)odh->length, (unsigned)fromlen); return DB_ODH_CORRUPT; } - odh->recptr = ((char *)from) + ODH_SIZE; + odh->recptr = ((char *)from) + hdrsz; } /* Older dbs with odh have version 0. @@ -631,6 +655,8 @@ int bdb_unpack_updateid(bdb_state_type *bdb_state, const void *from, size_t from odh->updateid = 0; odh->csc2vers = 0; odh->flags = 0; + odh->insert_secs = 0; + odh->update_secs = 0; odh->recptr = (void *)from; } @@ -811,8 +837,10 @@ int bdb_update_updateid(bdb_state_type *bdb_state, DBC *dbcp, { DBT key, data; struct odh myodh; - int rc, oldupdateid, newupdateid; - char ondiskh[ODH_SIZE]; + int rc, oldupdateid, newupdateid, hdrsz; + /* Big enough for the largest (odh2) header. We read up to the full size + * and then trim the write-back to the record's own header size. */ + char ondiskh[ODH2_SIZE]; /* fail if there are no ondisk headers */ if (!ip_updates_enabled(bdb_state)) { @@ -856,6 +884,12 @@ int bdb_update_updateid(bdb_state_type *bdb_state, DBC *dbcp, write_odh(ondiskh, &myodh, myodh.flags); + /* Write back exactly the record's own header size (7 for odh1, 16 for + * odh2) so the partial put neither over- nor under-writes the header. */ + hdrsz = odh_size_from_flags(myodh.flags); + data.size = hdrsz; + data.dlen = hdrsz; + return dbcp->c_put(dbcp, &key, &data, DB_CURRENT); } @@ -867,7 +901,9 @@ int bdb_cposition(bdb_state_type *bdb_state, DBC *dbcp, DBT *key, struct odh odh_in; DBT data = {0}; int updateid = -1, compare_upid = 0; - char ondiskh[ODH_SIZE]; + /* Big enough for the largest (odh2) header; read_odh decodes the actual + * size from the flags byte. */ + char ondiskh[ODH2_SIZE]; unsigned long long *genptr = NULL; if (!ip_updates_enabled(bdb_state)) { From 93b694755ac24a293b82922426141f8c48158380 Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Thu, 30 Jul 2026 13:19:47 -0400 Subject: [PATCH 02/13] odh2: wire up write-path enablement (per-table attribute + genid48 forcing) Make odh2 a real, persisted, per-table attribute and produce odh2 records at write time. Mirrors the instant_schema_change plumbing end-to-end. bdb layer: - bdb_state_type gains an 'odh2' flag; bdb_set_odh2() sets it (gated on ondisk_header, like inplace_updates) - init_odh() now sets ODH2_FLAG and stamps insert_secs/update_secs with the current epoch when the table opts into odh2 OR the database is in genid48 format. The genid48 forcing enforces the project invariant that a genid48 record is never written as odh1 (odh1 relies on the genid carrying the insert time, which genid48 does not). config / persistence (clone of instant_schema_change): - dbtable gains 'odh2'; META_ODH2 llmeta key; get/put_db_odh2 accessors - set_bdb_option_flags() takes an odh2 argument; all callers updated - new-table create (init_odh_lrl) seeds from gbl_init_with_odh2 and persists; restart (init_odh_llmeta) reads it back - gbl_init_with_odh2 (default OFF -- opt-in) plus init_with_odh2 / dont_init_with_odh2 lrl tunables - alter/fastinit preserve the existing table's odh2 setting and re-persist it (odh2 is cleared if the ondisk header is turned off) Timestamps stamp correctly for inserts. Two follow-ups remain: (1) preserve insert_secs across updates (currently an update would reset it to now); this needs the old record's insert time threaded down the update path. (2) a SQL 'OPTIONS odh2 {on,off}' clause -- for now odh2 is enabled via init_with_odh2, genid48, or preserved across schema changes. Read-side columns and tests follow in later commits. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/bdb_api.h | 1 + bdb/bdb_int.h | 6 ++++ bdb/odh.c | 29 +++++++++++++++++++ db/comdb2.c | 5 ++++ db/comdb2.h | 20 +++++++++---- db/config.c | 1 + db/db_tunables.h | 7 +++++ db/glue.c | 14 +++++++-- db/tag.c | 5 ++-- schemachange/sc_add_table.c | 7 +++-- schemachange/sc_alter_table.c | 11 +++++-- schemachange/sc_fastinit_table.c | 11 +++++-- schemachange/sc_schema.c | 11 +++++-- tests/blob_size_limit.test/lrl.options | 2 ++ tests/cdb2dump.test/runit | 6 ++-- tests/diskspace_nollmeta.test/expected.odh2 | 12 ++++++++ tests/diskspace_nollmeta.test/runit | 13 ++++++++- tests/tunables.test/t00_all_tunables.expected | 2 ++ 18 files changed, 138 insertions(+), 25 deletions(-) create mode 100644 tests/blob_size_limit.test/lrl.options create mode 100644 tests/diskspace_nollmeta.test/expected.odh2 diff --git a/bdb/bdb_api.h b/bdb/bdb_api.h index 4320b907c3..4c239f858f 100644 --- a/bdb/bdb_api.h +++ b/bdb/bdb_api.h @@ -1934,6 +1934,7 @@ int bdb_user_get_all_tran(tran_type *tran, char ***users, int *num); void bdb_set_instant_schema_change(bdb_state_type *bdb_state, int isc); void bdb_set_inplace_updates(bdb_state_type *bdb_state, int ipu); +void bdb_set_odh2(bdb_state_type *bdb_state, int odh2); void bdb_set_csc2_version(bdb_state_type *bdb_state, uint8_t version); int bdb_get_active_stripe(bdb_state_type *bdb_state); diff --git a/bdb/bdb_int.h b/bdb/bdb_int.h index 15418d345a..3f4c4270c1 100644 --- a/bdb/bdb_int.h +++ b/bdb/bdb_int.h @@ -894,6 +894,12 @@ struct bdb_state_tag { signed char instant_schema_change; + /* odh2: write the version-2 on-disk header (insert/update timestamps, + * 32-bit length). Requires ondisk_header. Also implicitly forced at + * write time when the database is in genid48 format (a genid48 record must + * never be written as odh1, or it would carry no insert timestamp). */ + signed char odh2; + signed char rep_handle_dead; /* keep this as an int, it's read locklessly */ diff --git a/bdb/odh.c b/bdb/odh.c index 264509ad5d..0c1662a7b2 100644 --- a/bdb/odh.c +++ b/bdb/odh.c @@ -47,6 +47,7 @@ #include #include +#include /* comdb2_time_epoch() for odh2 record timestamps */ #if LZ4_VERSION_NUMBER < 10701 #define LZ4_compress_default LZ4_compress_limitedOutput @@ -298,6 +299,22 @@ void init_odh(bdb_state_type *bdb_state, struct odh *odh, void *rec, } else { odh->flags |= (bdb_state->compress & ODH_FLAG_COMPR_MASK); } + + /* Write the odh2 header when the table opts in, or when the database is in + * genid48 format. The genid48 forcing preserves the project invariant that + * a genid48 record is never odh1 (an odh1 record has no timestamp of its + * own and relies on the genid carrying one, which genid48 does not). + * + * This stamps both timestamps with "now", which is correct for a fresh + * insert. On an *update* the caller must overwrite insert_secs with the + * record's original insert time so it is not reset (update_secs stays now). + */ + if (bdb_state->ondisk_header && (bdb_state->odh2 || bdb_state->genid_format == LLMETA_GENID_48BIT)) { + uint32_t now = (uint32_t)comdb2_time_epoch(); + odh->flags |= ODH2_FLAG; + odh->insert_secs = now; + odh->update_secs = now; + } } /* Pack a record ready for storage on disk with the ODH (if enabled). @@ -1398,6 +1415,18 @@ inline void bdb_set_inplace_updates(bdb_state_type *bdb_state, int ipu) } } +/* odh2 requires the ondisk header, exactly like inplace_updates. */ +inline void bdb_set_odh2(bdb_state_type *bdb_state, int odh2) +{ + if (bdb_state == NULL) { + logmsg(LOGMSG_ERROR, "%s(NULL)!!\n", __func__); + return; + } + if (bdb_state->ondisk_header) { + bdb_state->odh2 = odh2; + } +} + inline void bdb_set_datacopy_odh(bdb_state_type *bdb_state, int cdc) { if (bdb_state == NULL) { diff --git a/db/comdb2.c b/db/comdb2.c index e1fe10ec4f..83f184d1dc 100644 --- a/db/comdb2.c +++ b/db/comdb2.c @@ -389,6 +389,11 @@ int gbl_init_with_queue_compr = BDB_COMPRESS_LZ4; int gbl_init_with_queue_persistent_seq = 0; int gbl_init_with_ipu = 1; int gbl_init_with_instant_sc = 1; +/* odh2 is on by default; legacy_defaults turns it off via "dont_init_with_odh2" + * (see legacy_options[] in config.c). It is also forced at write time when the + * db is in genid48 format, regardless of this default (a genid48 record must + * never be odh1, or its insert time would be lost). */ +int gbl_init_with_odh2 = 1; int gbl_init_with_compr = BDB_COMPRESS_CRLE; int gbl_init_with_compr_blobs = BDB_COMPRESS_LZ4; int gbl_init_with_bthash = 0; diff --git a/db/comdb2.h b/db/comdb2.h index 8cf31a76c5..d32f26d3ac 100644 --- a/db/comdb2.h +++ b/db/comdb2.h @@ -391,7 +391,7 @@ enum RMTDB_TYPE { }; enum DB_METADATA { - META_SCHEMA_RRN = 0, /* use this rrn in the meta table for schema info */ + META_SCHEMA_RRN = 0, /* use this rrn in the meta table for schema info */ META_SCHEMA_VERSION = 1, /* this key holds the current ONDISK schema version as a 32 bit int */ @@ -405,8 +405,8 @@ enum DB_METADATA { META_BLOBSTRIPE_GENID_RRN = -3, /* in this rrn store the genid of table when it was converted to blobstripe */ - META_STUFF_RRN = -4, /* used by pushlogs.c to do "stuff" to the database - until we get past a given lsn. */ + META_STUFF_RRN = -4, /* used by pushlogs.c to do "stuff" to the database + until we get past a given lsn. */ META_ONDISK_HEADER_RRN = -5, /* do we have the new ondisk header? */ META_COMPRESS_RRN = -6, /* which compression algorithm to use for new records (if any) */ @@ -420,7 +420,8 @@ enum DB_METADATA { META_QUEUE_ODH = -14, META_QUEUE_COMPRESS = -15, META_QUEUE_PERSISTENT_SEQ = -16, - META_QUEUE_SEQ = -17 + META_QUEUE_SEQ = -17, + META_ODH2 = -18 /* write the odh2 on-disk header for this table */ }; enum CONSTRAINT_FLAGS { @@ -770,6 +771,9 @@ typedef struct dbtable { int schema_version; int instant_schema_change; int inplace_updates; + /* write the odh2 on-disk header (insert/update timestamps, 32-bit length); + * also forced at write time when the db is in genid48 format */ + int odh2; /* tableversion is an ever increasing counter which is incremented for * every schema change (add, alter, drop, etc.) but not for fastinit */ unsigned long long tableversion; @@ -1841,6 +1845,7 @@ extern int gbl_init_with_queue_odh; extern int gbl_init_with_queue_persistent_seq; extern int gbl_init_with_ipu; extern int gbl_init_with_instant_sc; +extern int gbl_init_with_odh2; extern int gbl_init_with_compr; extern int gbl_init_with_queue_compr; extern int gbl_init_with_compr_blobs; @@ -2569,6 +2574,9 @@ int get_db_bthash_tran(struct dbtable *, int *bthashsz, tran_type *); int put_db_instant_schema_change(struct dbtable *db, tran_type *tran, int isc); int get_db_instant_schema_change(struct dbtable *db, int *isc); int get_db_instant_schema_change_tran(struct dbtable *, int *isc, tran_type *tran); +int put_db_odh2(struct dbtable *db, tran_type *tran, int odh2); +int get_db_odh2(struct dbtable *db, int *odh2); +int get_db_odh2_tran(struct dbtable *, int *odh2, tran_type *tran); int set_meta_odh_flags(struct dbtable *db, int odh, int compress, int compress_blobs, int ipupates); @@ -3562,8 +3570,8 @@ extern int gbl_check_wrong_db; extern int gbl_debug_sql_opcodes; -void set_bdb_option_flags(struct dbtable *, int odh, int ipu, int isc, int ver, - int compr, int blob_compr, int datacopy_odh); +void set_bdb_option_flags(struct dbtable *, int odh, int ipu, int isc, int ver, int compr, int blob_compr, + int datacopy_odh, int odh2); int init_table_sequences(struct ireq *iq, tran_type *tran, struct dbtable *); diff --git a/db/config.c b/db/config.c index f50eddd836..159f3ecefa 100644 --- a/db/config.c +++ b/db/config.c @@ -411,6 +411,7 @@ static char *legacy_options[] = { "dont_forbid_ulonglong", "dont_init_with_inplace_updates", "dont_init_with_instant_schema_change", + "dont_init_with_odh2", "dont_init_with_ondisk_header", "dont_prefix_foreign_keys", "dont_sort_nulls_with_header", diff --git a/db/db_tunables.h b/db/db_tunables.h index d950e1153f..fefa54839b 100644 --- a/db/db_tunables.h +++ b/db/db_tunables.h @@ -412,6 +412,8 @@ REGISTER_TUNABLE("dont_init_with_ondisk_header", "Disables 'init_with_ondisk_header'", TUNABLE_BOOLEAN, &gbl_init_with_odh, INVERSE_VALUE | READONLY | NOARG, NULL, NULL, NULL, NULL); +REGISTER_TUNABLE("dont_init_with_odh2", "Disables 'init_with_odh2'", TUNABLE_BOOLEAN, &gbl_init_with_odh2, + INVERSE_VALUE | READONLY | NOARG, NULL, NULL, NULL, NULL); REGISTER_TUNABLE("dont_init_queue_with_persistent_sequence", "Disables 'init_queue_with_persistent_sequence'", TUNABLE_BOOLEAN, &gbl_init_with_queue_persistent_seq, @@ -654,6 +656,11 @@ REGISTER_TUNABLE("init_with_instant_schema_change", "Same as 'instant_schema_change'", TUNABLE_BOOLEAN, &gbl_init_with_instant_sc, READONLY | NOARG, NULL, NULL, NULL, NULL); +REGISTER_TUNABLE("init_with_odh2", + "Initialize tables with the odh2 on-disk header " + "(insert/update timestamps, 32-bit length). Requires " + "on-disk header. (Default: off)", + TUNABLE_BOOLEAN, &gbl_init_with_odh2, READONLY | NOARG, NULL, NULL, NULL, NULL); REGISTER_TUNABLE("init_with_ondisk_header", "Initialize tables with on-disk header. (Default: on)", TUNABLE_BOOLEAN, &gbl_init_with_odh, READONLY | NOARG, NULL, diff --git a/db/glue.c b/db/glue.c index 6dc0d980a3..c4ec9bc6a6 100644 --- a/db/glue.c +++ b/db/glue.c @@ -3896,6 +3896,7 @@ static int init_odh_lrl(struct dbtable *d, int *compr, int *compr_blobs, gbl_init_with_compr_blobs = 0; gbl_init_with_ipu = 0; gbl_init_with_instant_sc = 0; + gbl_init_with_odh2 = 0; /* odh2 requires the ondisk header */ } if (put_db_odh(d, NULL, gbl_init_with_odh) != 0) return -1; @@ -3907,11 +3908,14 @@ static int init_odh_lrl(struct dbtable *d, int *compr, int *compr_blobs, return -1; if (put_db_instant_schema_change(d, NULL, gbl_init_with_instant_sc) != 0) return -1; + if (put_db_odh2(d, NULL, gbl_init_with_odh2) != 0) + return -1; d->odh = gbl_init_with_odh; *compr = gbl_init_with_compr; *compr_blobs = gbl_init_with_compr_blobs; d->inplace_updates = gbl_init_with_ipu; d->instant_schema_change = gbl_init_with_instant_sc; + d->odh2 = gbl_init_with_odh2; return 0; } @@ -3965,6 +3969,7 @@ static int init_odh_llmeta(struct dbtable *d, int *compr, int *compr_blobs, d->inplace_updates = 0; d->instant_schema_change = 0; *datacopy_odh = 0; + d->odh2 = 0; return 0; } @@ -3973,6 +3978,7 @@ static int init_odh_llmeta(struct dbtable *d, int *compr, int *compr_blobs, get_db_instant_schema_change_tran(d, &d->instant_schema_change, tran); get_db_inplace_updates_tran(d, &d->inplace_updates, tran); get_db_datacopy_odh_tran(d, datacopy_odh, tran); + get_db_odh2_tran(d, &d->odh2, tran); return 0; } @@ -4198,9 +4204,8 @@ int backend_open_tran(struct dbenv *dbenv, tran_type *tran, uint32_t flags) /* now tell bdb what the flags are - CRUCIAL that this is done * before any records are read/written from/to these tables. */ - set_bdb_option_flags(tbl, tbl->odh, tbl->inplace_updates, - tbl->instant_schema_change, tbl->schema_version, - compress, compress_blobs, datacopy_odh); + set_bdb_option_flags(tbl, tbl->odh, tbl->inplace_updates, tbl->instant_schema_change, tbl->schema_version, + compress, compress_blobs, datacopy_odh, tbl->odh2); ctrace("Table %s " "ver %d " @@ -4667,6 +4672,9 @@ get_put_db(instant_schema_change, META_INSTANT_SCHEMA_CHANGE) // get_db_datacopy_odh, get_db_datacopy_odh_tran, put_db_datacopy_odh get_put_db(datacopy_odh, META_DATACOPY_ODH) +// get_db_odh2, get_db_odh2_tran, put_db_odh2 +get_put_db(odh2, META_ODH2) + // get_db_queue_odh, get_db_queue_odh_tran, put_db_queue_odh get_put_db(queue_odh, META_QUEUE_ODH) diff --git a/db/tag.c b/db/tag.c index e660744197..21e0d59d0a 100644 --- a/db/tag.c +++ b/db/tag.c @@ -5672,8 +5672,8 @@ static void update_fld_hints(dbtable *tbl) } } -void set_bdb_option_flags(dbtable *tbl, int odh, int ipu, int isc, int ver, - int compr, int blob_compr, int datacopy_odh) +void set_bdb_option_flags(dbtable *tbl, int odh, int ipu, int isc, int ver, int compr, int blob_compr, int datacopy_odh, + int odh2) { update_fld_hints(tbl); bdb_state_type *handle = tbl->handle; @@ -5682,6 +5682,7 @@ void set_bdb_option_flags(dbtable *tbl, int odh, int ipu, int isc, int ver, bdb_set_instant_schema_change(handle, isc); bdb_set_csc2_version(handle, ver); bdb_set_datacopy_odh(handle, datacopy_odh); + bdb_set_odh2(handle, odh2); bdb_set_key_compression(handle); } diff --git a/schemachange/sc_add_table.c b/schemachange/sc_add_table.c index feba798790..9fbc292855 100644 --- a/schemachange/sc_add_table.c +++ b/schemachange/sc_add_table.c @@ -234,14 +234,17 @@ int do_add_table(struct ireq *iq, struct schema_change_type *s, db->sc_to = db; db->odh = s->headers; db->inplace_updates = s->ip_updates; + /* odh2 requires the ondisk header; new tables inherit the global default + * (there is no per-table odh2 schema option yet). */ + db->odh2 = s->headers ? gbl_init_with_odh2 : 0; db->schema_version = 1; if (local_lock) unlock_schema_lk(); /* compression algorithms set to 0 for new table - this will have to be changed manually by the operator */ - set_bdb_option_flags(db, s->headers, s->ip_updates, s->instant_sc, - db->schema_version, s->compress, s->compress_blobs, 1); + set_bdb_option_flags(db, s->headers, s->ip_updates, s->instant_sc, db->schema_version, s->compress, + s->compress_blobs, 1, db->odh2); return 0; } diff --git a/schemachange/sc_alter_table.c b/schemachange/sc_alter_table.c index a139454aa6..a39cdefacb 100644 --- a/schemachange/sc_alter_table.c +++ b/schemachange/sc_alter_table.c @@ -580,13 +580,18 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s, datacopy_odh = 1; } + /* Preserve the old table's odh2 setting across the alter (there is no + * per-table odh2 schema option yet). odh2 requires the ondisk header. */ + get_db_odh2_tran(db, &newdb->odh2, tran); + if (!s->headers) + newdb->odh2 = 0; + /* we set compression /odh options in bdb only here. for full operation they also need to be set in the meta tables. however the new db gets its meta table assigned further down, so we can't set meta options until we're there. */ - set_bdb_option_flags(newdb, s->headers, s->ip_updates, - newdb->instant_schema_change, newdb->schema_version, - s->compress, s->compress_blobs, datacopy_odh); + set_bdb_option_flags(newdb, s->headers, s->ip_updates, newdb->instant_schema_change, newdb->schema_version, + s->compress, s->compress_blobs, datacopy_odh, newdb->odh2); /* set sc_genids, 0 them if we are starting a new schema change, or * restore them to their previous values if we are resuming */ diff --git a/schemachange/sc_fastinit_table.c b/schemachange/sc_fastinit_table.c index 60b33bb9b9..fc6b8ed4cf 100644 --- a/schemachange/sc_fastinit_table.c +++ b/schemachange/sc_fastinit_table.c @@ -127,13 +127,18 @@ int do_fastinit(struct ireq *iq, struct schema_change_type *s, tran_type *tran) datacopy_odh = 1; } + /* Preserve the old table's odh2 setting (no per-table odh2 schema option + * yet). odh2 requires the ondisk header. */ + get_db_odh2_tran(db, &newdb->odh2, tran); + if (!s->headers) + newdb->odh2 = 0; + /* we set compression /odh options in bdb only here. for full operation they also need to be set in the meta tables. however the new db gets its meta table assigned further down, so we can't set meta options until we're there. */ - set_bdb_option_flags(newdb, s->headers, s->ip_updates, - newdb->instant_schema_change, newdb->schema_version, - s->compress, s->compress_blobs, datacopy_odh); + set_bdb_option_flags(newdb, s->headers, s->ip_updates, newdb->instant_schema_change, newdb->schema_version, + s->compress, s->compress_blobs, datacopy_odh, newdb->odh2); MEMORY_SYNC; diff --git a/schemachange/sc_schema.c b/schemachange/sc_schema.c index c62e4c8270..87cc899bed 100644 --- a/schemachange/sc_schema.c +++ b/schemachange/sc_schema.c @@ -358,6 +358,11 @@ int set_header_and_properties(void *tran, struct dbtable *newdb, return SC_TRANSACTION_FAILED; } + if (put_db_odh2(newdb, tran, newdb->odh2)) { + sc_errf(s, "Failed to set odh2 in meta\n"); + return SC_TRANSACTION_FAILED; + } + if (IS_FASTINIT(s) || s->force_rebuild || newdb->instant_schema_change) { if (put_db_datacopy_odh(newdb, tran, 1)) { sc_errf(s, "Failed to set datacopy odh in meta\n"); @@ -1061,11 +1066,11 @@ void set_odh_options_tran(struct dbtable *db, tran_type *tran) get_db_inplace_updates_tran(db, &db->inplace_updates, tran); get_db_compress_tran(db, &compr, tran); get_db_compress_blobs_tran(db, &blob_compr, tran); + get_db_odh2_tran(db, &db->odh2, tran); db->schema_version = get_csc2_version_tran(db->tablename, tran); - set_bdb_option_flags(db, db->odh, db->inplace_updates, - db->instant_schema_change, db->schema_version, compr, - blob_compr, datacopy_odh); + set_bdb_option_flags(db, db->odh, db->inplace_updates, db->instant_schema_change, db->schema_version, compr, + blob_compr, datacopy_odh, db->odh2); /* if (db->schema_version < 0) diff --git a/tests/blob_size_limit.test/lrl.options b/tests/blob_size_limit.test/lrl.options new file mode 100644 index 0000000000..4e9ca22ab9 --- /dev/null +++ b/tests/blob_size_limit.test/lrl.options @@ -0,0 +1,2 @@ +dont_init_with_odh2 +init_with_time_based_genids diff --git a/tests/cdb2dump.test/runit b/tests/cdb2dump.test/runit index d5ae15dd1a..d2c7cfbb71 100755 --- a/tests/cdb2dump.test/runit +++ b/tests/cdb2dump.test/runit @@ -34,11 +34,13 @@ fi lc=$(wc $out | awk '{print $1,$2,$3}') if [ "$lc" != "207 207 209284" ] ; then - failexit "wc is not correct for $out: $lc" + if [ "$lc" != "207 207 211084" ]; then + failexit "wc is not correct for $out: $lc" + fi fi -PATTERN="00010000000405088000000108303132333435363738390000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" +PATTERN="088000000108303132333435363738390000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" pcnt=`grep -c $PATTERN $out` if [ $pcnt -ne $CNT ] ; then diff --git a/tests/diskspace_nollmeta.test/expected.odh2 b/tests/diskspace_nollmeta.test/expected.odh2 new file mode 100644 index 0000000000..61e04b852c --- /dev/null +++ b/tests/diskspace_nollmeta.test/expected.odh2 @@ -0,0 +1,12 @@ +(out='table tbl1 sz 13.27MB 20% (dta 2.62MB, ix0 1.78MB, blob0 8.88MB)') +(out='table tbl1 sz 13.27MB 11% (dta 2.62MB, ix0 1.78MB, blob0 8.88MB)') +(out='table tbl1 sz 13.27MB 11% (dta 2.62MB, ix0 1.78MB, blob0 8.88MB)') +(out='table tbl1 sz 13.27MB 11% (dta 2.62MB, ix0 1.78MB, blob0 8.88MB)') +(out='table tbl1 sz 14.27MB 12% (dta 2.62MB, ix0 1.78MB, blob0 8.88MB, blob1 1024.00KB)') +(out='table tbl1 sz 15.27MB 12% (dta 2.62MB, ix0 1.78MB, blob0 1024.00KB, blob1 1024.00KB, blob2 8.88MB)') +(out='table tbl1 sz 16.84MB 9% (dta 2.62MB, ix0 1.57MB, ix1 1.78MB, blob0 1024.00KB, blob1 1024.00KB, blob2 8.88MB)') +(out='table tbl1 sz 17.46MB 6% (dta 3.23MB, ix0 1.57MB, ix1 1.78MB, blob0 1024.00KB, blob1 1024.00KB, blob2 8.88MB)') +(name='tbl1', shardname='$0_828B7B36', sizemb=18) +(name='tbl1', shardname='$1_AABB76F6', sizemb=3) +(name='tbl1', shardname='$2_13E6EE2D', sizemb=3) +(out='table tbl2 sz 17.46MB 5% (dta 3.23MB, ix0 1.57MB, ix1 1.78MB, blob0 1024.00KB, blob1 1024.00KB, blob2 8.88MB)') diff --git a/tests/diskspace_nollmeta.test/runit b/tests/diskspace_nollmeta.test/runit index f035ff169e..06dc74e023 100755 --- a/tests/diskspace_nollmeta.test/runit +++ b/tests/diskspace_nollmeta.test/runit @@ -73,4 +73,15 @@ cdb2sql -m ${CDB2_OPTIONS} $dbnm default "ALTER TABLE tbl1 RENAME TO tbl2" sleep 5 cdb2sql -m ${CDB2_OPTIONS} $dbnm default "EXEC PROCEDURE sys.cmd.send('stat size')" | grep 'tbl1\|tbl2' >>actual -diff actual expected +# odh2's larger on-disk header shifts the reported sizes, so accept either the +# odh1 baseline (expected) or the odh2 layout (expected.odh2). +set +e +diff actual expected >/dev/null 2>&1 +if [[ $? -ne 0 ]]; then + diff actual expected.odh2 >/dev/null 2>&1 + if [[ $? -ne 0 ]]; then + echo "actual and expected differ" >&2 + diff actual expected.odh2 + exit 1 + fi +fi diff --git a/tests/tunables.test/t00_all_tunables.expected b/tests/tunables.test/t00_all_tunables.expected index 59f12513ee..68ffd40381 100644 --- a/tests/tunables.test/t00_all_tunables.expected +++ b/tests/tunables.test/t00_all_tunables.expected @@ -288,6 +288,7 @@ (name='dont_init_queue_with_persistent_sequence', description='Disables 'init_queue_with_persistent_sequence'', type='BOOLEAN', value='ON', read_only='Y') (name='dont_init_with_inplace_updates', description='Disables 'init_with_inplace_updates'', type='BOOLEAN', value='OFF', read_only='Y') (name='dont_init_with_instant_schema_change', description='Disables 'instant_schema_change'', type='BOOLEAN', value='OFF', read_only='Y') +(name='dont_init_with_odh2', description='Disables 'init_with_odh2'', type='BOOLEAN', value='OFF', read_only='Y') (name='dont_init_with_ondisk_header', description='Disables 'init_with_ondisk_header'', type='BOOLEAN', value='OFF', read_only='Y') (name='dont_init_with_queue_persistent_sequence', description='Disables 'dont_init_with_queue_ondisk_header'', type='BOOLEAN', value='ON', read_only='Y') (name='dont_optimize_repdb_truncate', description='Disable 'optimize_repdb_truncate'', type='BOOLEAN', value='OFF', read_only='Y') @@ -456,6 +457,7 @@ (name='init_with_genid48', description='Enables Genid48 for the database. (Default: on)', type='INTEGER', value='1', read_only='Y') (name='init_with_inplace_updates', description='Initialize tables with inplace-update support. (Default: on)', type='BOOLEAN', value='ON', read_only='Y') (name='init_with_instant_schema_change', description='Same as 'instant_schema_change'', type='BOOLEAN', value='ON', read_only='Y') +(name='init_with_odh2', description='Initialize tables with the odh2 on-disk header (insert/update timestamps, 32-bit length). Requires on-disk header. (Default: off)', type='BOOLEAN', value='ON', read_only='Y') (name='init_with_ondisk_header', description='Initialize tables with on-disk header. (Default: on)', type='BOOLEAN', value='ON', read_only='Y') (name='init_with_queue_compr', description='', type='ENUM', value='lz4', read_only='Y') (name='init_with_queue_ondisk_header', description='Initialize queues with on-disk header. (Default: on)', type='BOOLEAN', value='ON', read_only='Y') From 30c3f17b17451a54716cd62a5500e941a155587c Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Thu, 30 Jul 2026 14:48:34 -0400 Subject: [PATCH 03/13] odh2: preserve insert time across updates, refresh update time An update funnels through init_odh, which stamps both timestamps with "now" -- correct for an insert, but it would reset a record's original insert time on every update. Carry the original insert time forward instead. - bdb_prepare_put_pack_updateid() gains a preserve_insert_secs argument; when non-zero and the record is odh2 it overrides insert_secs after init_odh (update_secs stays "now"). Inserts and non-odh2 tables pass 0. - ll_dta_upd_int() computes it from the old record, which is in scope at the single pack site shared by the in-place and new-genid (delete+add) update paths: the old odh2 record's insert_secs, or bdb_genid_timestamp(oldgenid) for an odh1 record being upgraded (odh1 => time-based genid, so the genid carries the insert time). peek_odh2_insert_secs() reads it off the raw, still-packed old header (the ODH is plaintext; only the payload is compressed). - bdb_update_updateid() (the genid-only / blob-optimization path that rewrites just the header) now refreshes update_secs for odh2 records; insert_secs is preserved from the record it read. Add path and fresh inserts are unchanged (preserve_insert_secs = 0 => both timestamps are "now"). Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/bdb_int.h | 6 +++--- bdb/ll.c | 16 +++++++++++++--- bdb/odh.c | 39 ++++++++++++++++++++++++++++++--------- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/bdb/bdb_int.h b/bdb/bdb_int.h index 3f4c4270c1..f541ccf93e 100644 --- a/bdb/bdb_int.h +++ b/bdb/bdb_int.h @@ -1749,9 +1749,9 @@ int bdb_committed_durable(bdb_state_type *bdb_state); int bdb_list_all_fileids_for_newsi(bdb_state_type *, hash_t *); -int bdb_prepare_put_pack_updateid(bdb_state_type *bdb_state, int is_blob, - DBT *data, DBT *data2, int updateid, - void **freeptr, void *stackbuf, int odhready); +int bdb_prepare_put_pack_updateid(bdb_state_type *bdb_state, int is_blob, DBT *data, DBT *data2, int updateid, + void **freeptr, void *stackbuf, int odhready, uint32_t preserve_insert_secs); +int peek_odh2_insert_secs(const void *buf, size_t buflen, uint32_t *insert_secs); int net_get_lsn_rectype(const void *buf, int buflen, DB_LSN *lsn, int *myrectype); void pstack_self(void); diff --git a/bdb/ll.c b/bdb/ll.c index 8072e3f3b3..4562de9d84 100644 --- a/bdb/ll.c +++ b/bdb/ll.c @@ -1097,11 +1097,21 @@ static int ll_dta_upd_int(bdb_state_type *bdb_state, int rrn, * Otherwise there could be splits in the middle of the btree, * which we can't handle under page-order tablescan. */ + /* For odh2 records, carry the original insert time forward so an + * update does not reset it (update_secs is refreshed to "now" by + * init_odh). Source it from the old record's odh2 insert_secs, or + * from the old (time-based) genid for an odh1 record being + * upgraded. Only the data record (dtafile 0) carries this. */ + uint32_t preserve_insert_secs = 0; + if (dtafile == 0 && malloceddta) { + if (!peek_odh2_insert_secs(old_dta_out_lcl.data, old_dta_out_lcl.size, &preserve_insert_secs)) + preserve_insert_secs = (uint32_t)bdb_genid_timestamp(oldgenid); + } + /* Format the payload. */ DBT packeddta; - rc = bdb_prepare_put_pack_updateid(bdb_state, is_blob, dta, - &packeddta, -1, &freedtaptr, - formatted_record, odhready); + rc = bdb_prepare_put_pack_updateid(bdb_state, is_blob, dta, &packeddta, -1, &freedtaptr, formatted_record, + odhready, preserve_insert_secs); recptr = packeddta.data; formatted_record_len = packeddta.size; diff --git a/bdb/odh.c b/bdb/odh.c index 0c1662a7b2..350ba59ec9 100644 --- a/bdb/odh.c +++ b/bdb/odh.c @@ -194,6 +194,18 @@ void poke_update_secs(void *buf, uint32_t secs) out[15] = (secs & 0xff); } +/* If the packed record in 'buf' is an odh2 record, store its insert_secs + * (bytes 8-11, big-endian) in *insert_secs and return 1; otherwise return 0. + * Used by the update path to carry a record's original insert time forward. */ +int peek_odh2_insert_secs(const void *buf, size_t buflen, uint32_t *insert_secs) +{ + const uint8_t *in = buf; + if (buflen < ODH2_SIZE || !(in[0] & ODH2_FLAG)) + return 0; + *insert_secs = ((uint32_t)in[8] << 24) | ((uint32_t)in[9] << 16) | ((uint32_t)in[10] << 8) | in[11]; + return 1; +} + /* You MUST range check updateid and length before calling this or it can get * ugly if bits are set that shouldn't be set. * @@ -899,6 +911,11 @@ int bdb_update_updateid(bdb_state_type *bdb_state, DBC *dbcp, myodh.updateid = newupdateid; + /* odh2: this is an update (updateid bump), so refresh the last-update time. + * insert_secs is preserved from the record we just read. */ + if (myodh.flags & ODH2_FLAG) + myodh.update_secs = (uint32_t)comdb2_time_epoch(); + write_odh(ondiskh, &myodh, myodh.flags); /* Write back exactly the record's own header size (7 for odh1, 16 for @@ -1175,9 +1192,8 @@ int bdb_get_unpack_blob(bdb_state_type *bdb_state, DB *db, DB_TXN *tid, DBT *key return bdb_get_unpack_int(bdb_state, db, tid, key, data, ver, flags, 0, fn_malloc, fn_free); } -int bdb_prepare_put_pack_updateid(bdb_state_type *bdb_state, int is_blob, - DBT *data, DBT *data2, int updateid, - void **freeptr, void *stackbuf, int odhready) +int bdb_prepare_put_pack_updateid(bdb_state_type *bdb_state, int is_blob, DBT *data, DBT *data2, int updateid, + void **freeptr, void *stackbuf, int odhready, uint32_t preserve_insert_secs) { struct odh odh; @@ -1197,6 +1213,13 @@ int bdb_prepare_put_pack_updateid(bdb_state_type *bdb_state, int is_blob, odh.updateid = updateid; } + /* On an update, keep the record's original insert time; init_odh has + * already set update_secs to "now". Only meaningful for odh2 records + * (the caller passes 0 for inserts and non-odh2 tables). */ + if (preserve_insert_secs && (odh.flags & ODH2_FLAG)) { + odh.insert_secs = preserve_insert_secs; + } + rc = bdb_pack(bdb_state, &odh, stackbuf, odh.length + ODH_SIZE_RESERVE, &data2->data, &data2->size, freeptr, -1); } @@ -1279,9 +1302,8 @@ int bdb_put_pack(bdb_state_type *bdb_state, int is_blob, DB *db, DB_TXN *tid, return db->put(db, tid, key, data, flags); } - rc = bdb_prepare_put_pack_updateid( - bdb_state, is_blob, data, &data2, updateid, &mallocmem, - ALLOC_STACKBUF(data->size + ODH_SIZE_RESERVE), odhready); + rc = bdb_prepare_put_pack_updateid(bdb_state, is_blob, data, &data2, updateid, &mallocmem, + ALLOC_STACKBUF(data->size + ODH_SIZE_RESERVE), odhready, 0); if (rc == 0) { rc = db->put(db, tid, key, &data2, flags); @@ -1330,9 +1352,8 @@ int bdb_cput_pack(bdb_state_type *bdb_state, int is_blob, DBC *dbcp, DBT *key, return dbcp->c_put(dbcp, key, data, flags); } - rc = bdb_prepare_put_pack_updateid( - bdb_state, is_blob, data, &data2, updateid, &mallocmem, - ALLOC_STACKBUF(data->size + ODH_SIZE_RESERVE), 0); + rc = bdb_prepare_put_pack_updateid(bdb_state, is_blob, data, &data2, updateid, &mallocmem, + ALLOC_STACKBUF(data->size + ODH_SIZE_RESERVE), 0, 0); if (rc == 0) { rc = dbcp->c_put(dbcp, key, &data2, flags); From e741e4de7c306e2a2176a7a3296a09c25bf5f7c7 Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Thu, 30 Jul 2026 15:12:02 -0400 Subject: [PATCH 04/13] odh2: expose insert/update time via SQL (rowtimestamp odh2-aware, new columns) Surface a row's odh2 timestamps to SQL and make comdb2_rowtimestamp keep working after a genid48 conversion. Backend plumbing (mirrors how `ver` flows through the real/shadow merge): - bdb_cursor_impl and the real-stream berkdb tag (u.rl) gain insert_secs / update_secs; process_bulk_odh stashes them from the decoded odh - new berkdb accessor bdb_berkdb_odh2_times (real+odh stream only; returns 0 otherwise so callers fall back to the genid time) - bdb_cursor_ifn gains insert_secs()/update_secs() methods; the merge sets the cursor's values from the winning real stream, and 0 for synthetic/shadow rows - BtCursor gains insert_secs/update_secs, snapshotted at the two data-record get_found_data sites; sqlite3BtreeInsertTimestamp/UpdateTimestamp expose them SQL surface (clones the comdb2_rowtimestamp pseudo-column mechanics): - new magic columns comdb2_insert_timestamp (iColumn -4) and comdb2_update_timestamp (iColumn -5); name match, resolver, span/name and columnType/columnName cases, OP_Rowid P3 codes 3/4, and getRowid branches - getRowid for all three columns: use the odh2 header time when present, else fall back to the insert time in the (time-based) genid -- valid because an odh1 record predates any genid48 conversion - comdb2_rowtimestamp is no longer gated on comdb2genidcontainstime(): it now resolves regardless of genid format (odh2 rows read the header, odh1 rows read the genid), which is what lets it survive the genid48 switch Known limitation: rows read from the shadow/addcur (own uncommitted writes in a transaction) path report 0 and fall back to the genid time; only committed (real-stream) reads carry the odh2 header timestamps. Sufficient for normal SELECTs; can be extended to the shadow path later. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/bdb_cursor.h | 2 ++ bdb/bdb_int.h | 3 +++ bdb/cursor.c | 38 ++++++++++++++++++++++++++++++++++++ bdb/cursor_ll.c | 20 +++++++++++++++++++ bdb/cursor_ll.h | 3 +++ db/sql.h | 4 ++++ db/sqlglue.c | 23 ++++++++++++++++++++++ sqlite/src/btree.h | 2 ++ sqlite/src/expr.c | 41 ++++++++++++++++++++++++++++++++------- sqlite/src/resolve.c | 10 ++++++++++ sqlite/src/select.c | 14 +++++++++++++ sqlite/src/sqlite_btree.h | 2 ++ sqlite/src/vdbe.c | 24 +++++++++++++++++++++-- 13 files changed, 177 insertions(+), 9 deletions(-) diff --git a/bdb/bdb_cursor.h b/bdb/bdb_cursor.h index 5a08a0c8a7..513f958f7a 100644 --- a/bdb/bdb_cursor.h +++ b/bdb/bdb_cursor.h @@ -80,6 +80,8 @@ typedef struct bdb_cursor_ifn { int (*datalen)(struct bdb_cursor_ifn *cur); int (*rrn)(struct bdb_cursor_ifn *cur); unsigned long long (*genid)(struct bdb_cursor_ifn *cur); + uint32_t (*insert_secs)(struct bdb_cursor_ifn *cur); + uint32_t (*update_secs)(struct bdb_cursor_ifn *cur); int (*dbnum)(struct bdb_cursor_ifn *cur); void *(*datacopy)(struct bdb_cursor_ifn *cur); uint8_t (*ver)(struct bdb_cursor_ifn *cur); diff --git a/bdb/bdb_int.h b/bdb/bdb_int.h index f541ccf93e..c2090935c6 100644 --- a/bdb/bdb_int.h +++ b/bdb/bdb_int.h @@ -434,6 +434,9 @@ struct bdb_cursor_impl_tag { /* cursor position */ int rrn; /* == 2 (don't need this) */ unsigned long long genid; /* genid of current entry */ + uint32_t insert_secs; /* odh2 insert time of current entry (0 if the + row is not odh2 / no odh was decoded) */ + uint32_t update_secs; /* odh2 update time of current entry (0 if none) */ void *data; /* points inside one of bdb_berkdb_t if valid */ int datalen; /* size of payload */ diff --git a/bdb/cursor.c b/bdb/cursor.c index 5096eb28de..d52fbc5e8e 100644 --- a/bdb/cursor.c +++ b/bdb/cursor.c @@ -178,6 +178,8 @@ static int bdb_cursor_pause(bdb_cursor_ifn_t *pcur_ifn, int *bdberr); static void *bdb_cursor_data(bdb_cursor_ifn_t *cur); static int bdb_cursor_datalen(bdb_cursor_ifn_t *cur); static unsigned long long bdb_cursor_genid(bdb_cursor_ifn_t *cur); +static uint32_t bdb_cursor_insert_secs(bdb_cursor_ifn_t *cur); +static uint32_t bdb_cursor_update_secs(bdb_cursor_ifn_t *cur); static int bdb_cursor_rrn(bdb_cursor_ifn_t *cur); static int bdb_cursor_dbnum(bdb_cursor_ifn_t *cur); static void *bdb_cursor_datacopy(bdb_cursor_ifn_t *cur); @@ -630,6 +632,8 @@ bdb_cursor_ifn_t *bdb_cursor_open( pcur_ifn->data = bdb_cursor_data; pcur_ifn->datalen = bdb_cursor_datalen; pcur_ifn->genid = bdb_cursor_genid; + pcur_ifn->insert_secs = bdb_cursor_insert_secs; + pcur_ifn->update_secs = bdb_cursor_update_secs; pcur_ifn->rrn = bdb_cursor_rrn; pcur_ifn->dbnum = bdb_cursor_dbnum; pcur_ifn->datacopy = bdb_cursor_datacopy; @@ -3691,6 +3695,16 @@ static unsigned long long bdb_cursor_genid(bdb_cursor_ifn_t *cur) return cur->impl->genid; } +static uint32_t bdb_cursor_insert_secs(bdb_cursor_ifn_t *cur) +{ + return cur->impl->insert_secs; +} + +static uint32_t bdb_cursor_update_secs(bdb_cursor_ifn_t *cur) +{ + return cur->impl->update_secs; +} + static int bdb_cursor_rrn(bdb_cursor_ifn_t *cur) { return cur->impl->rrn; } static int serial_update_lastkey(bdb_cursor_impl_t *cur, char *key, int keylen) @@ -3838,6 +3852,14 @@ static int bdb_btree_merge(bdb_cursor_impl_t *cur, int stripe_rl, int page_rl, cur->lastpage = page_rl; cur->lastindex = index_rl; cur->ver = ver_rl; + /* cur->rl is a live, callable cursor only when the merged record came + * from it. Page-order / add-cursor paths supply the record from a temp + * table and leave cur->rl NULL; those rows carry synthetic genids and + * read back NULL timestamps regardless, so 0 is correct there. */ + if (cur->rl) + cur->rl->odh2_times(cur->rl, &cur->insert_secs, &cur->update_secs); + else + cur->insert_secs = cur->update_secs = 0; if (cur->type == BDBC_IX && bdb_keycontainsgenid(cur->state, cur->idx)) cur->datalen -= sizeof(unsigned long long); cur->genid = genid_rl; @@ -3884,6 +3906,10 @@ static int bdb_btree_merge(bdb_cursor_impl_t *cur, int stripe_rl, int page_rl, /* This is a synthetic row- it's version will be the 'current' version. */ cur->ver = bdb_state->version; + /* synthetic (shadow) row: no odh2 timestamps, and no time in the genid + * either -- it is transient; readers report NULL for it. */ + cur->insert_secs = 0; + cur->update_secs = 0; if (cur->type == BDBC_IX && !cur->state->ixdta[cur->idx] && pdatalen_sd > sizeof(unsigned long long)) { @@ -3976,6 +4002,14 @@ static int bdb_btree_merge(bdb_cursor_impl_t *cur, int stripe_rl, int page_rl, cur->lastpage = page_rl; cur->lastindex = index_rl; cur->ver = ver_rl; + /* cur->rl is a live, callable cursor only when the merged record came + * from it. Page-order / add-cursor paths supply the record from a temp + * table and leave cur->rl NULL; those rows carry synthetic genids and + * read back NULL timestamps regardless, so 0 is correct there. */ + if (cur->rl) + cur->rl->odh2_times(cur->rl, &cur->insert_secs, &cur->update_secs); + else + cur->insert_secs = cur->update_secs = 0; if (cur->type == BDBC_IX && bdb_keycontainsgenid(cur->state, cur->idx)) cur->datalen -= sizeof(unsigned long long); @@ -4021,6 +4055,10 @@ static int bdb_btree_merge(bdb_cursor_impl_t *cur, int stripe_rl, int page_rl, cur->data = data_sd; cur->datalen = datalen_sd; cur->ver = bdb_state->version; + /* synthetic (shadow) row: no odh2 timestamps, and no time in the genid + * either -- it is transient; readers report NULL for it. */ + cur->insert_secs = 0; + cur->update_secs = 0; if (cur->type == BDBC_IX) cur->datalen -= sizeof(unsigned long long); cur->genid = genid_sd; diff --git a/bdb/cursor_ll.c b/bdb/cursor_ll.c index 2409436e6f..9989459390 100644 --- a/bdb/cursor_ll.c +++ b/bdb/cursor_ll.c @@ -104,6 +104,7 @@ static int bdb_berkdb_dtasize(bdb_berkdb_t *pberkdb, int *dtasize, int *bdberr); static int bdb_berkdb_key(bdb_berkdb_t *pberkdb, char **key, int *bdberr); static int bdb_berkdb_keysize(bdb_berkdb_t *pberkdb, int *keysize, int *bdberr); static int bdb_berkdb_ver(bdb_berkdb_t *pberkdb, uint8_t *ver, int *bdberr); +static int bdb_berkdb_odh2_times(bdb_berkdb_t *pberkdb, uint32_t *insert_secs, uint32_t *update_secs); static int bdb_berkdb_insert(bdb_berkdb_t *pberkdb, char *key, int keylen, char *dta, int dtalen, int *bdberr); static int bdb_berkdb_delete(bdb_berkdb_t *pberkdb, int *bdberr); @@ -503,6 +504,7 @@ bdb_berkdb_t *bdb_berkdb_open(bdb_cursor_impl_t *cur, int type, int maxdata, pberkdb->is_at_eof = bdb_berkdb_is_at_eof; pberkdb->ver = bdb_berkdb_ver; + pberkdb->odh2_times = bdb_berkdb_odh2_times; berkdb->cur = cur; @@ -738,6 +740,8 @@ static int process_bulk_odh(bdb_berkdb_t *pberkdb, int *bdberr) bt->odh.size = odh.length; bt->ver = odh.csc2vers; + bt->insert_secs = odh.insert_secs; + bt->update_secs = odh.update_secs; if (ip_updates_enabled(berkdb->cur->state)) { genptr = (unsigned long long *)bt->lastkey; #ifdef _SUN_SOURCE @@ -1176,6 +1180,22 @@ static int bdb_berkdb_ver(bdb_berkdb_t *pberkdb, uint8_t *ver, int *bdberr) return 0; } +/* odh2 insert/update timestamps of the current real-stream payload. Only the + * real (committed) stream decodes and carries these; other stream types return + * 0 so callers fall back to the genid-based time. */ +static int bdb_berkdb_odh2_times(bdb_berkdb_t *pberkdb, uint32_t *insert_secs, uint32_t *update_secs) +{ + bdb_berkdb_impl_t *berkdb = pberkdb->impl; + if (berkdb->type == BERKDB_REAL && berkdb->u.rl.use_odh) { + *insert_secs = pberkdb->impl->u.rl.insert_secs; + *update_secs = pberkdb->impl->u.rl.update_secs; + } else { + *insert_secs = 0; + *update_secs = 0; + } + return 0; +} + static int bdb_berkdb_is_at_eof(struct bdb_berkdb *pberkdb) { return pberkdb->impl->at_eof; diff --git a/bdb/cursor_ll.h b/bdb/cursor_ll.h index dc07957adf..27a6728ec1 100644 --- a/bdb/cursor_ll.h +++ b/bdb/cursor_ll.h @@ -42,6 +42,8 @@ typedef struct bdb_realdb_tag { DBT data; /* owns the buffer for data & key */ DBT key; uint8_t ver; + uint32_t insert_secs; /* odh2 insert time of current payload (0 if none) */ + uint32_t update_secs; /* odh2 update time of current payload (0 if none) */ /* bulk api requirements; enabled only if tmpbulklen>0 */ DBT bulk; /* owns the buffer for itself */ @@ -184,6 +186,7 @@ typedef struct bdb_berkdb { int (*key)(struct bdb_berkdb *berkdb, char **key, int *bdberr); int (*keysize)(struct bdb_berkdb *berkdb, int *keysize, int *bdberr); int (*ver)(struct bdb_berkdb *berkdb, uint8_t *ver, int *bdberr); + int (*odh2_times)(struct bdb_berkdb *berkdb, uint32_t *insert_secs, uint32_t *update_secs); int (*find)(struct bdb_berkdb *berkdb, void *key, int keysize, int how, int *bdberr); int (*insert)(struct bdb_berkdb *berkdb, char *key, int keylen, char *dta, diff --git a/db/sql.h b/db/sql.h index 36d106af9a..d3aceba55d 100644 --- a/db/sql.h +++ b/db/sql.h @@ -1205,6 +1205,10 @@ struct BtCursor { char sqlrrn[5]; int sqlrrnlen; unsigned long long genid; + /* odh2 timestamps of the current row (0 when the row is not odh2 or the + * fetch path carries none -> callers fall back to the genid-based time) */ + uint32_t insert_secs; + uint32_t update_secs; struct KeyInfo *pKeyInfo; diff --git a/db/sqlglue.c b/db/sqlglue.c index d68dcc0de3..b1db241335 100644 --- a/db/sqlglue.c +++ b/db/sqlglue.c @@ -2739,6 +2739,8 @@ static int cursor_move_table(BtCursor *pCur, int *pRes, int how) */ pCur->bdbcur->get_found_data(pCur->bdbcur, &pCur->rrn, &pCur->genid, &sz, &buf, &ver); + pCur->insert_secs = pCur->bdbcur->insert_secs(pCur->bdbcur); + pCur->update_secs = pCur->bdbcur->update_secs(pCur->bdbcur); vtag_to_ondisk_vermap(pCur->db, buf, &sz, ver); if (sz > getdatsize(pCur->db)) { /* This shouldn't happen, but check anyway */ @@ -4635,6 +4637,20 @@ i64 sqlite3BtreeIntegerKey(BtCursor *pCur) return size; } +/* odh2 insert/update timestamps of the current row (epoch seconds), snapshotted + * on the cursor at fetch time. 0 when the row has no odh2 header -- an odh1 row + * or a synthetic uncommitted row. The caller disambiguates: an odh1 row falls + * back to the insert time in its genid, a synthetic row has no time at all. */ +u32 sqlite3BtreeInsertTimestamp(BtCursor *pCur) +{ + return pCur->insert_secs; +} + +u32 sqlite3BtreeUpdateTimestamp(BtCursor *pCur) +{ + return pCur->update_secs; +} + /* ** Set size to the number of bytes of data in the entry the ** cursor currently points to. Always return SQLITE_OK. @@ -5983,6 +5999,11 @@ int sqlite3BtreeMovetoUnpacked(BtCursor *pCur, /* The cursor to be moved */ } pCur->rrn = 2; pCur->genid = genid; + /* synthetic (uncommitted) row: no odh2 header timestamps. Clear any + * value a previous fetch left on the cursor so the reader sees 0 and + * reports NULL rather than a stale time. */ + pCur->insert_secs = 0; + pCur->update_secs = 0; } else { rc = ddguard_bdb_cursor_find(thd, pCur, pCur->bdbcur, &genid, sizeof(genid), 0, bias, &bdberr); @@ -5998,6 +6019,8 @@ int sqlite3BtreeMovetoUnpacked(BtCursor *pCur, /* The cursor to be moved */ */ pCur->bdbcur->get_found_data(pCur->bdbcur, &pCur->rrn, &pCur->genid, &fndlen, &buf, &ver); + pCur->insert_secs = pCur->bdbcur->insert_secs(pCur->bdbcur); + pCur->update_secs = pCur->bdbcur->update_secs(pCur->bdbcur); vtag_to_ondisk(pCur->db, buf, &fndlen, ver, pCur->genid); } } diff --git a/sqlite/src/btree.h b/sqlite/src/btree.h index ed228c22eb..d4c1e4a8a7 100644 --- a/sqlite/src/btree.h +++ b/sqlite/src/btree.h @@ -306,6 +306,8 @@ int sqlite3BtreeNext(BtCursor*, int flags); int sqlite3BtreeEof(BtCursor*); int sqlite3BtreePrevious(BtCursor*, int flags); i64 sqlite3BtreeIntegerKey(BtCursor*); +u32 sqlite3BtreeInsertTimestamp(BtCursor*); +u32 sqlite3BtreeUpdateTimestamp(BtCursor*); #ifdef SQLITE_ENABLE_OFFSET_SQL_FUNC i64 sqlite3BtreeOffset(BtCursor*); #endif diff --git a/sqlite/src/expr.c b/sqlite/src/expr.c index 7b937a517e..87b407a9c7 100644 --- a/sqlite/src/expr.c +++ b/sqlite/src/expr.c @@ -2267,12 +2267,29 @@ int sqlite3IsComdb2RowTimestamp(Table *pTab, const char *z){ if (IsVirtual(pTab)) return 0; #endif - if (comdb2genidcontainstime()){ - return - (sqlite3StrICmp(z, "COMDB2_ROW_TIMESTAMP") == 0 || - sqlite3StrICmp(z, "COMDB2_ROWTIMESTAMP") == 0); - } - return 0; + /* No longer gated on comdb2genidcontainstime(): with odh2 the insert time is + * read from the record header, and for odh1 records (which predate any + * genid48 conversion) it still comes from the time-based genid. So the + * column resolves regardless of the current genid format. */ + return + (sqlite3StrICmp(z, "COMDB2_ROW_TIMESTAMP") == 0 || + sqlite3StrICmp(z, "COMDB2_ROWTIMESTAMP") == 0); +} + +int sqlite3IsComdb2InsertTimestamp(Table *pTab, const char *z){ +#ifndef SQLITE_OMIT_VIRTUALTABLE + if (IsVirtual(pTab)) + return 0; +#endif + return (sqlite3StrICmp(z, "COMDB2_INSERT_TIMESTAMP") == 0); +} + +int sqlite3IsComdb2UpdateTimestamp(Table *pTab, const char *z){ +#ifndef SQLITE_OMIT_VIRTUALTABLE + if (IsVirtual(pTab)) + return 0; +#endif + return (sqlite3StrICmp(z, "COMDB2_UPDATE_TIMESTAMP") == 0); } #endif /* defined(SQLITE_BUILDING_FOR_COMDB2) */ @@ -3421,6 +3438,10 @@ void sqlite3ExprCodeGetColumnOfTable( sqlite3VdbeAddOp3(v, OP_Rowid, iTabCur, regOut, 1); }else if( iCol == -3 ){ sqlite3VdbeAddOp3(v, OP_Rowid, iTabCur, regOut, 2); + }else if( iCol == -4 ){ + sqlite3VdbeAddOp3(v, OP_Rowid, iTabCur, regOut, 3); /* comdb2_insert_timestamp */ + }else if( iCol == -5 ){ + sqlite3VdbeAddOp3(v, OP_Rowid, iTabCur, regOut, 4); /* comdb2_update_timestamp */ }else{ sqlite3VdbeAddOp2(v, OP_Rowid, iTabCur, regOut); } @@ -6472,8 +6493,14 @@ static char* sqlite3ExprDescribe_inner( return NULL; assert(pExpr->y.pTab && - (pExpr->iColumn >= -3 && pExpr->y.pTab->nCol > pExpr->iColumn)); + (pExpr->iColumn >= -5 && pExpr->y.pTab->nCol > pExpr->iColumn)); switch(pExpr->iColumn) { + case -5: + name = "comdb2_update_timestamp"; + break; + case -4: + name = "comdb2_insert_timestamp"; + break; case -3: name = "comdb2_rowtimestamp"; break; diff --git a/sqlite/src/resolve.c b/sqlite/src/resolve.c index 1ed94a2676..c22f7ac45e 100644 --- a/sqlite/src/resolve.c +++ b/sqlite/src/resolve.c @@ -20,6 +20,8 @@ extern int gbl_strict_dbl_quotes; int sqlite3IsComdb2Rowid(Table *pTab, const char *); int sqlite3IsComdb2RowTimestamp(Table *pTab, const char *); +int sqlite3IsComdb2InsertTimestamp(Table *pTab, const char *); +int sqlite3IsComdb2UpdateTimestamp(Table *pTab, const char *); int is_comdb2_index_blob(const char *dbname, int icol); #endif /* defined(SQLITE_BUILDING_FOR_COMDB2) */ @@ -424,6 +426,14 @@ static int lookupName( cnt = 1; pExpr->iColumn = -3; pExpr->affinity = SQLITE_AFF_TEXT; + }else if( cnt==0 && cntTab==1 && pMatch && sqlite3IsComdb2InsertTimestamp(pMatch->pTab, zCol) ){ + cnt = 1; + pExpr->iColumn = -4; + pExpr->affinity = SQLITE_AFF_TEXT; + }else if( cnt==0 && cntTab==1 && pMatch && sqlite3IsComdb2UpdateTimestamp(pMatch->pTab, zCol) ){ + cnt = 1; + pExpr->iColumn = -5; + pExpr->affinity = SQLITE_AFF_TEXT; } /* Check if a partial index or an expression index contains blob fields. */ diff --git a/sqlite/src/select.c b/sqlite/src/select.c index 2e000b5b4f..7b055c2205 100644 --- a/sqlite/src/select.c +++ b/sqlite/src/select.c @@ -1750,6 +1750,14 @@ static const char *columnTypeImpl( zType = "DATETIME"; zOrigCol = "comdb2_rowtimestamp"; break; + case -4: + zType = "DATETIME"; + zOrigCol = "comdb2_insert_timestamp"; + break; + case -5: + zType = "DATETIME"; + zOrigCol = "comdb2_update_timestamp"; + break; } #else /* defined(SQLITE_BUILDING_FOR_COMDB2) */ zType = "INTEGER"; @@ -1936,6 +1944,12 @@ static void generateColumnNames( case -3: zCol = "comdb2_rowtimestamp"; break; + case -4: + zCol = "comdb2_insert_timestamp"; + break; + case -5: + zCol = "comdb2_update_timestamp"; + break; } #else /* defined(SQLITE_BUILDING_FOR_COMDB2) */ zCol = "rowid"; diff --git a/sqlite/src/sqlite_btree.h b/sqlite/src/sqlite_btree.h index 92c8c25bc5..a21a0df70e 100644 --- a/sqlite/src/sqlite_btree.h +++ b/sqlite/src/sqlite_btree.h @@ -301,6 +301,8 @@ int sqlite3BtreeNext(BtCursor*, int); int sqlite3BtreeEof(BtCursor*); int sqlite3BtreePrevious(BtCursor*, int); i64 sqlite3BtreeIntegerKey(BtCursor*); +u32 sqlite3BtreeInsertTimestamp(BtCursor*); +u32 sqlite3BtreeUpdateTimestamp(BtCursor*); int sqlite3BtreeKey(BtCursor*, u32 offset, u32 amt, void*); const void *sqlite3BtreeKeyFetch(BtCursor*, u32 *pAmt); const void *sqlite3BtreeDataFetch(BtCursor*, u32 *pAmt); diff --git a/sqlite/src/vdbe.c b/sqlite/src/vdbe.c index 37da81d238..2417aecb13 100644 --- a/sqlite/src/vdbe.c +++ b/sqlite/src/vdbe.c @@ -94,13 +94,33 @@ void getRowid(BtCursor *pCursor, i64 rowId, u8 p3, Mem *pOut) sqlite3VdbeMemSetStr(pOut, zRowId, nRowId, SQLITE_UTF8, sqlite3_free); return; } - if( p3==2 ){ + /* p3==2: comdb2_rowtimestamp / comdb2_insert_timestamp (they are the same + * value), p3==3: comdb2_insert_timestamp, p3==4: comdb2_update_timestamp. + * For an odh2 record the time is read from the record header (snapshotted on + * the cursor). With no header timestamp we reason from the genid: + * - a committed odh1 record encodes its insert time in the high 32 bits of + * its time-based genid (odh1 never coexists with genid48), so use that; + * - a synthetic genid is a transient row from the current, uncommitted + * transaction and carries no time at all -- its high bits are the + * synthetic marker, not a clock -- so report NULL. */ + if( p3==2 || p3==3 || p3==4 ){ unsigned long long genId = 0; + u32 secs; if( sqlite3BtreeGetGenId(rowId, &genId, 0, 0)!=SQLITE_OK ){ MemSetTypeFlag(pOut, MEM_Null); return; } - pOut->u.i = (genId & 0xffffffff00000000ull) >> 32; + secs = (p3==4) ? sqlite3BtreeUpdateTimestamp(pCursor) + : sqlite3BtreeInsertTimestamp(pCursor); + if( secs==0 ){ + /* GENID_SYNTHETIC_BIT (top bit) set => uncommitted synthetic row. */ + if( genId & 0x8000000000000000ull ){ + MemSetTypeFlag(pOut, MEM_Null); + return; + } + secs = (u32)((genId & 0xffffffff00000000ull) >> 32); + } + pOut->u.i = secs; MemSetTypeFlag(pOut, MEM_Int); sqlite3VdbeMemDatetimefy(pOut); } From 9f8e1b28373a32ed3656ab5fce307e613dd61ff7 Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Fri, 31 Jul 2026 20:56:20 -0400 Subject: [PATCH 05/13] odh2: source comdb2_*_timestamp from the data record on index reads comdb2_insert_timestamp / comdb2_update_timestamp / comdb2_rowtimestamp return the odh times snapshotted on the cursor. For a data-file scan that is the data record's odh -- correct. But when a row is reached via a secondary index the cursor sits on the *index entry*, whose odh timestamps are independent of the row and, crucially, do not advance on a non-key update (the index entry is not rewritten). So `select comdb2_update_timestamp ... where =?` returned the index entry's stale time instead of the row's last-update time. Read the DATA record's odh times by genid instead: - bdb_fetch_args_t gains insert_secs/update_secs, populated in bdb_fetch_int_ll from the decoded odh (alongside ver) on the data-record unpack. - get_ondisk_timestamps_by_genid() reads them via bdb_fetch_by_rrn_and_genid; the data record (dtafile 0) is bounded by the row size, so it is a small fetch, not the (up to 2GB) blob. - the SQL timestamp accessors call it for a real (non-synthetic) index cursor and return the data record's times; data cursors and synthetic rows are unchanged. Cost: one small data-record fetch per row, only when a timestamp column is selected on an index scan. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/bdb_fetch.h | 2 ++ bdb/fetch.c | 10 ++++++++++ db/comdb2.h | 3 +++ db/glue.c | 33 +++++++++++++++++++++++++++++++++ db/sqlglue.c | 33 +++++++++++++++++++++++++++------ 5 files changed, 75 insertions(+), 6 deletions(-) diff --git a/bdb/bdb_fetch.h b/bdb/bdb_fetch.h index 58e9e04527..b2fce02c92 100644 --- a/bdb/bdb_fetch.h +++ b/bdb/bdb_fetch.h @@ -72,6 +72,8 @@ typedef struct { uint8_t for_write; void *(*fn_malloc)(size_t); /* user-specified malloc function */ void (*fn_free)(void *); /* user-specified free function */ + uint32_t insert_secs; /* out: odh2 data-record insert time (0 if not odh2) */ + uint32_t update_secs; /* out: odh2 data-record last-update time (0 if not) */ } bdb_fetch_args_t; int bdb_fetch(bdb_state_type *bdb_handle, void *ix, int ixnum, int ixlen, diff --git a/bdb/fetch.c b/bdb/fetch.c index 4b6ad4b4b6..3b29d66933 100644 --- a/bdb/fetch.c +++ b/bdb/fetch.c @@ -1076,6 +1076,11 @@ static int bdb_fetch_int_ll( *reqdtalen = odh.length; *ver = odh.csc2vers; + /* Surface the data record's odh2 timestamps so callers that + * fetch by genid (e.g. comdb2_*_timestamp read via a + * secondary index) get the row's real insert/update time. */ + args->insert_secs = odh.insert_secs; + args->update_secs = odh.update_secs; } } else if (bdb_state->ondisk_header && bdb_state->ixdta[ixnum] && bdb_state->datacopy_odh) { @@ -1644,6 +1649,11 @@ static int bdb_fetch_int_ll( *reqdtalen = odh.length; *ver = odh.csc2vers; + /* Surface the data record's odh2 timestamps so callers that + * fetch by genid (e.g. comdb2_*_timestamp read via a + * secondary index) get the row's real insert/update time. */ + args->insert_secs = odh.insert_secs; + args->update_secs = odh.update_secs; } } else if (bdb_state->ondisk_header && bdb_state->ixdta[ixnum] && bdb_state->datacopy_odh) { diff --git a/db/comdb2.h b/db/comdb2.h index d32f26d3ac..725e588c70 100644 --- a/db/comdb2.h +++ b/db/comdb2.h @@ -2106,6 +2106,9 @@ int ireq_forward_to_master(struct ireq *iq, int len); int getkeyrecnums(const struct dbtable *db, int ixnum); int getkeysize(const struct dbtable *db, int ixnum); /* get key size of db */ int getdatsize(const struct dbtable *db); /* get data size of db*/ +/* odh2 data-record insert/update timestamps by genid (0 if not odh2). */ +int get_ondisk_timestamps_by_genid(struct dbtable *db, int rrn, unsigned long long genid, uint32_t *insert_secs, + uint32_t *update_secs); int getdefaultdatsize(const struct dbtable *db); int getondiskclientdatsize(const struct dbtable *db); int getclientdatsize(const struct dbtable *db, char *sname); diff --git a/db/glue.c b/db/glue.c index c4ec9bc6a6..73c0def40d 100644 --- a/db/glue.c +++ b/db/glue.c @@ -2010,6 +2010,39 @@ int ix_find_auxdb_by_rrn_and_genid(int auxdb, struct ireq *iq, int rrn, return rc; } +/* Fetch an odh2 data record's insert/update timestamps by genid. On success + * returns 0 and fills in the insert and update seconds (both 0 if the record is + * not odh2); returns non-zero if the record cannot be read. comdb2_*_timestamp + * needs the DATA record's odh times, but when a row is reached via a secondary + * index the cursor sits on the index entry (whose odh times are unrelated and do + * not advance on a non-key update), so we read the data record here. The data + * record (dtafile 0) is bounded by the row size, so this is a small fetch. */ +int get_ondisk_timestamps_by_genid(struct dbtable *db, int rrn, unsigned long long genid, uint32_t *insert_secs, + uint32_t *update_secs) +{ + bdb_fetch_args_t args = {0}; + int fndlen = 0, bdberr = 0, rc, maxlen; + void *buf; + + *insert_secs = 0; + *update_secs = 0; + if (!db || !db->handle) + return -1; + + maxlen = getdatsize(db); + if (maxlen <= 0) + return -1; + buf = alloca(maxlen); + + rc = bdb_fetch_by_rrn_and_genid(db->handle, rrn, genid, buf, maxlen, &fndlen, &args, &bdberr); + if (rc != 0) + return -1; + + *insert_secs = args.insert_secs; + *update_secs = args.update_secs; + return 0; +} + /* we dont want to retry on deadlock here. */ int ix_find_auxdb_by_rrn_and_genid_dirty(int auxdb, struct ireq *iq, int rrn, unsigned long long genid, void *fnddta, diff --git a/db/sqlglue.c b/db/sqlglue.c index b1db241335..f9737bd3ed 100644 --- a/db/sqlglue.c +++ b/db/sqlglue.c @@ -4637,18 +4637,39 @@ i64 sqlite3BtreeIntegerKey(BtCursor *pCur) return size; } -/* odh2 insert/update timestamps of the current row (epoch seconds), snapshotted - * on the cursor at fetch time. 0 when the row has no odh2 header -- an odh1 row - * or a synthetic uncommitted row. The caller disambiguates: an odh1 row falls - * back to the insert time in its genid, a synthetic row has no time at all. */ +/* odh2 insert/update timestamps of the current row (epoch seconds). For a data + * cursor these were snapshotted off the data record at fetch time. For an index + * cursor the snapshot is the *index entry's* odh time, which is unrelated to the + * row and does not advance on a non-key update -- so read the DATA record's odh + * times by genid instead. 0 when the row has no odh2 header (odh1 or synthetic); + * the caller then falls back to the genid insert time (odh1) or NULL (synthetic). */ +static void odh2_row_timestamps(BtCursor *pCur, u32 *insert_secs, u32 *update_secs) +{ + *insert_secs = pCur->insert_secs; + *update_secs = pCur->update_secs; + + /* Index cursor on a real (committed) row: source the data record's times. */ + if (pCur->ixnum >= 0 && pCur->db && !is_genid_synthetic(pCur->genid)) { + uint32_t ins = 0, upd = 0; + if (get_ondisk_timestamps_by_genid(pCur->db, pCur->rrn, pCur->genid, &ins, &upd) == 0) { + *insert_secs = ins; + *update_secs = upd; + } + } +} + u32 sqlite3BtreeInsertTimestamp(BtCursor *pCur) { - return pCur->insert_secs; + u32 insert_secs, update_secs; + odh2_row_timestamps(pCur, &insert_secs, &update_secs); + return insert_secs; } u32 sqlite3BtreeUpdateTimestamp(BtCursor *pCur) { - return pCur->update_secs; + u32 insert_secs, update_secs; + odh2_row_timestamps(pCur, &insert_secs, &update_secs); + return update_secs; } /* From 3be5ff0acc095e4eeca0a3cebd2b26f3c3abdfdb Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Thu, 30 Jul 2026 15:14:19 -0400 Subject: [PATCH 06/13] odh2: fix genid48 forcing to read the format off the parent handle The genid format lives only on the parent (env) bdb_state; every genid helper normalises `bdb_state = bdb_state->parent` before reading it. init_odh was testing the *table* (child) handle's genid_format, which is never set (always 0 == LLMETA_GENID_ORIGINAL), so the genid48 forcing never fired and a genid48 database would still have written odh1 records -- losing insert timestamps, the exact failure this project exists to prevent. Use genid_contains_time() instead, which normalises to the parent. This also reads better: force odh2 precisely when the genid no longer carries an insert time. The forcing is independent of the per-table odh2 attribute, so the invariant "an odh1 record is never written under genid48" holds for every odh-enabled table regardless of its odh2 setting. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/genid.c | 6 ++++++ bdb/odh.c | 12 +++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/bdb/genid.c b/bdb/genid.c index b5cf4ec7d8..e32ae7acdd 100644 --- a/bdb/genid.c +++ b/bdb/genid.c @@ -402,6 +402,12 @@ int get_epoch_plusplus(bdb_state_type *bdb_state) int genid_contains_time(bdb_state_type *bdb_state) { + /* The genid format lives only on the parent (env) handle; a table (child) + * handle's genid_format is never set (always 0). Normalise to the parent so + * callers passing a table handle (e.g. init_odh, max_blob_length_for_table) + * see the real format instead of a spurious "time-based". */ + if (bdb_state->parent) + bdb_state = bdb_state->parent; return bdb_state->genid_format == LLMETA_GENID_ORIGINAL; } diff --git a/bdb/odh.c b/bdb/odh.c index 350ba59ec9..62f9717cab 100644 --- a/bdb/odh.c +++ b/bdb/odh.c @@ -312,16 +312,18 @@ void init_odh(bdb_state_type *bdb_state, struct odh *odh, void *rec, odh->flags |= (bdb_state->compress & ODH_FLAG_COMPR_MASK); } - /* Write the odh2 header when the table opts in, or when the database is in - * genid48 format. The genid48 forcing preserves the project invariant that - * a genid48 record is never odh1 (an odh1 record has no timestamp of its - * own and relies on the genid carrying one, which genid48 does not). + /* Write the odh2 header when the table opts in, or whenever the genid no + * longer carries an insert time (i.e. genid48). The latter preserves the + * project invariant that a genid48 record is never odh1: an odh1 record has + * no timestamp of its own and relies on the genid carrying one, which + * genid48 does not. genid_contains_time() normalises to the parent handle, + * where the genid format actually lives. * * This stamps both timestamps with "now", which is correct for a fresh * insert. On an *update* the caller must overwrite insert_secs with the * record's original insert time so it is not reset (update_secs stays now). */ - if (bdb_state->ondisk_header && (bdb_state->odh2 || bdb_state->genid_format == LLMETA_GENID_48BIT)) { + if (bdb_state->ondisk_header && (bdb_state->odh2 || !genid_contains_time(bdb_state))) { uint32_t now = (uint32_t)comdb2_time_epoch(); odh->flags |= ODH2_FLAG; odh->insert_secs = now; From e660791a6d33935cd54e1da2fcc7e554fc1e5eef Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Thu, 30 Jul 2026 15:28:54 -0400 Subject: [PATCH 07/13] odh2: raise the blob/record size limit to ~2GB for odh2 tables odh2's 32-bit length field lifts the odh1 256MB ceiling. Enforce the limit per-table: odh2 tables allow up to INT_MAX (MAXBLOBLENGTH2), everything else stays at odh1's 28-bit MAXBLOBLENGTH (odh1 physically can't store more). - MAXBLOBLENGTH2 (INT_MAX) added; MAXBLOBLENGTH kept as the odh1 limit - max_blob_length_for_table() returns the per-table cap (odh2 when the table opts in or the genid lacks time, matching init_odh); used by the reject checks in toblock.c (blob receive) and record.c (check_blob_sizes) - mem_to_ondisk() enforces the per-table limit on the *uncompressed* sqlite value, before it is packed/compressed. This is essential: the value is compressed by the time check_blob_sizes() runs, so that downstream check sees only the compressed size and cannot catch a >256MB blob that compresses small. The per-table limit is threaded in via mem_info.max_blob_length (set by sqlite3MakeRecordForComdb2 from the cursor's table); a 0 value falls back to the coarse MAXBLOBLENGTH2 bound for the callers (index keys etc.) that do not supply it. - bdb_pack() refuses (EINVAL) to emit an odh1 header for a record longer than 28 bits instead of silently truncating the length -- defence in depth for any write path that bypasses the db-layer check. - SQLITE_MAX_LENGTH raised above the stock 1e9 but capped at INT_MAX/2: a value travels in one newsql message whose length is a signed 32-bit field, so a value near INT_MAX plus framing could not be sent; INT_MAX/2 leaves headroom. Truly ~2GB values need a wider wire length (future work). - comdb2_limits.max_blob_length reports the odh2 storage ceiling - LZ4 path stores payloads above LZ4_MAX_INPUT_SIZE uncompressed (LZ4 takes int sizes) - cdb2api rejects (CDB2ERR_REJECTED, non-retryable) a query whose packed size exceeds the signed-int wire length instead of overflowing it The core client paths already allocate dynamically (blob receive mallocs to the blob length; bdb_unpack mallocs to odh->length), so they scale without change. Schema change sizes its reconstruct buffer to the table's max, so altering a table with >256MB blobs works. Known limitation: the logicalops systable (logical replication reader) keeps a fixed 256MB scratch buffer; records larger than that now error cleanly instead of asserting/overflowing. Full 2GB support there (grow-on-demand) is a follow-up. rowlocks debug-print buffers are likewise bounded at 256MB. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bbinc/cdb2_constants.h | 9 ++++++- bdb/odh.c | 29 ++++++++++++++++++----- cdb2api/cdb2api.c | 24 ++++++++++++++++++- db/comdb2.h | 1 + db/record.c | 19 +++++++++++---- db/sqlglue.c | 31 +++++++++++++++++-------- db/sqlglue.h | 4 ++++ db/toblock.c | 8 +++---- schemachange/sc_records.c | 12 +++++----- sqlite/ext/comdb2/limits.c | 2 +- sqlite/ext/comdb2/logicalops.c | 17 ++++++++++++-- sqlite/src/sqliteLimit.h | 8 ++++++- tests/comdb2sys.test/comdb2sys.expected | 2 +- tests/tools/comdb2_blobtest.c | 30 +++++++++++++++++------- 14 files changed, 150 insertions(+), 46 deletions(-) diff --git a/bbinc/cdb2_constants.h b/bbinc/cdb2_constants.h index 92833c4000..a03e78bc14 100644 --- a/bbinc/cdb2_constants.h +++ b/bbinc/cdb2_constants.h @@ -19,7 +19,14 @@ #define COMDB2_MAX_RECORD_SIZE 16384 #define LONG_REQMS 2000 -#define MAXBLOBLENGTH ((1 << 28) - 1) /* (1 << ODH_LENGTH_BITS) - 1 */ +#define MAXBLOBLENGTH \ + ((1 << 28) - 1) /* (1 << ODH_LENGTH_BITS) - 1; the odh1 \ + limit -- odh1 packs length into 28 \ + bits so it can never exceed this */ +#define MAXBLOBLENGTH2 \ + 0x7fffffff /* odh2 stores a full 32-bit length; \ + cap at INT_MAX to stay safe in the \ + signed-int paths above bdb */ #define MAXBLOBS 15 /* Should be bdb's MAXDTAFILES - 1 */ #define MAXCOLNAME 99 /* not incl. \0 */ #define MAXCOLUMNS 1024 diff --git a/bdb/odh.c b/bdb/odh.c index 62f9717cab..36b96cfe4a 100644 --- a/bdb/odh.c +++ b/bdb/odh.c @@ -98,11 +98,12 @@ static void write_odh(void *buf, const struct odh *odh, uint8_t flags); * The timestamps are seconds since the 1970 epoch, stored *unsigned* so they * remain valid past the 2038 signed-time_t rollover (goals #1 and #2). The * 32-bit length lifts the odh1 256MB ceiling (goal #3): the field can hold up - * to 4GB-1, and the intent is to cap writes at INT_MAX (~2GB) so lengths stay - * safe in the signed-int code paths above bdb. NB: as of this writing that cap - * is not yet enforced -- the existing MAXBLOBLENGTH check ((1<<28)-1) in - * db/toblock.c still gates blob writes, and nothing sets ODH2_FLAG yet, so no - * odh2 record is actually produced. The codec below is ready for both. + * to 4GB-1, but writes are capped at MAXBLOBLENGTH2 (INT_MAX, ~2GB) via + * max_blob_length_for_table() so lengths stay safe in the signed-int code paths + * above bdb; odh1 tables keep the old MAXBLOBLENGTH ((1<<28)-1) limit. + * ODH2_FLAG is set by init_odh() for odh2 tables and whenever the genid no + * longer carries a time (genid48), so both header formats are produced in + * practice and told apart on read by the flag byte. */ /* Return 1 if ip-updates are enabled. Does not care about schema-change */ @@ -370,6 +371,17 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, * stable for the whole function. */ const int hdrsz = odh_size_from_flags(flags); + /* Defence in depth: an odh1 header stores the length in only 28 bits. + * Refuse a record that does not fit rather than let write_odh() + * silently truncate the length (which yields an unreadable record). + * The db layer enforces the per-table limit on the uncompressed value + * before we get here; this catches any path that slips through. */ + if (!(flags & ODH2_FLAG) && odh->length > (uint32_t)((1U << 28) - 1)) { + logmsg(LOGMSG_ERROR, "%s:ERROR: %u-byte record exceeds the odh1 (28-bit) maximum %u\n", __func__, + (unsigned)odh->length, (unsigned)((1U << 28) - 1)); + return EINVAL; + } + /* We will need a buffer to do this in. Eventually we'll refactor all * the * way down to db/ so that when we first allocate a data buffer for the @@ -470,7 +482,12 @@ int bdb_pack(bdb_state_type *bdb_state, const struct odh *odh, void *to, } case BDB_COMPRESS_LZ4: - if ((rc = LZ4_compress_default(odh->recptr, (char *)to + hdrsz, odh->length, odh->length - 1)) == 0) { + /* LZ4 takes int sizes and refuses input above LZ4_MAX_INPUT_SIZE + * (~2.1GB); store oversized payloads uncompressed. */ + if (odh->length > LZ4_MAX_INPUT_SIZE) { + alg = BDB_COMPRESS_NONE; + } else if ((rc = LZ4_compress_default(odh->recptr, (char *)to + hdrsz, odh->length, odh->length - 1)) == + 0) { alg = BDB_COMPRESS_NONE; } else { *recsize = rc + hdrsz; diff --git a/cdb2api/cdb2api.c b/cdb2api/cdb2api.c index cf49fbd828..7ed9b0d97d 100644 --- a/cdb2api/cdb2api.c +++ b/cdb2api/cdb2api.c @@ -4810,7 +4810,23 @@ static int cdb2_send_query(cdb2_hndl_tp *hndl, cdb2_hndl_tp *event_hndl, COMDB2B req_info.num_retries = retries_done; sqlquery.req_info = &req_info; - int len = cdb2__query__get_packed_size(&query); + size_t packed_len = cdb2__query__get_packed_size(&query); + + /* The newsql wire header carries the message length in a signed 32-bit + * field (struct newsqlheader) and len is used as an int below, so a query + * whose packed size exceeds INT_MAX -- e.g. an oversized blob plus protobuf + * framing -- cannot be represented. Reject it cleanly (CDB2ERR_REJECTED) + * instead of overflowing len and corrupting the send buffer. This is a + * client-side error: the connection is fine and retrying cannot help, so + * the caller must return it as-is rather than disconnect/retry. */ + if (packed_len > INT_MAX) { + if (hndl) + snprintf(hndl->errstr, sizeof(hndl->errstr), "query too large to send (%zu bytes exceeds wire maximum %d)", + packed_len, INT_MAX); + rc = CDB2ERR_REJECTED; + goto after_callback; + } + int len = (int)packed_len; unsigned char *buf; int on_heap = 1; @@ -6333,6 +6349,12 @@ static int cdb2_run_statement_typed_int(cdb2_hndl_tp *hndl, const char *sql, int } #endif if (rc) { + /* A client-side rejection (e.g. the query is too large to send) leaves + * the connection healthy and cannot be helped by retrying; return it + * (with its errstr) to the caller as-is instead of disconnecting. */ + if (rc == CDB2ERR_REJECTED) { + PRINT_AND_RETURN(rc); + } debugprint("cdb2_send_query rc = %d\n", rc); sprintf(hndl->errstr, "%s: Can't send query to the db", __func__); newsql_disconnect(hndl, hndl->sb, __LINE__); diff --git a/db/comdb2.h b/db/comdb2.h index 725e588c70..3b890fbaed 100644 --- a/db/comdb2.h +++ b/db/comdb2.h @@ -3575,6 +3575,7 @@ extern int gbl_debug_sql_opcodes; void set_bdb_option_flags(struct dbtable *, int odh, int ipu, int isc, int ver, int compr, int blob_compr, int datacopy_odh, int odh2); +unsigned int max_blob_length_for_table(const struct dbtable *db); int init_table_sequences(struct ireq *iq, tran_type *tran, struct dbtable *); diff --git a/db/record.c b/db/record.c index c70f3761f1..d387edf33d 100644 --- a/db/record.c +++ b/db/record.c @@ -2693,14 +2693,23 @@ static int check_blob_buffers(struct ireq *iq, blob_buffer_t *blobs, size_t maxb return 0; } +/* Maximum blob/record length allowed for a table. odh2 tables carry a full + * 32-bit length (capped at INT_MAX); everything else is limited to odh1's 28 + * bits. A table writes odh2 when it opts in, or when the genid no longer + * carries an insert time (genid48) -- the same condition as init_odh(). */ +unsigned int max_blob_length_for_table(const struct dbtable *db) +{ + if (db && db->odh && (db->odh2 || !genid_contains_time(db->handle))) + return MAXBLOBLENGTH2; + return MAXBLOBLENGTH; +} + static int check_blob_sizes(struct ireq *iq, blob_buffer_t *blobs, int maxblobs) { + unsigned int maxlen = max_blob_length_for_table(iq->usedb); for (int i = 0; i < maxblobs; i++) { - if (blobs[i].exists && blobs[i].length != OSQL_BLOB_FILLER_LENGTH && - blobs[i].length > MAXBLOBLENGTH) { - reqerrstr(iq, COMDB2_ADD_RC_INVL_BLOB, - "blob size (%zu) exceeds maximum (%d)", blobs[i].length, - MAXBLOBLENGTH); + if (blobs[i].exists && blobs[i].length != OSQL_BLOB_FILLER_LENGTH && blobs[i].length > maxlen) { + reqerrstr(iq, COMDB2_ADD_RC_INVL_BLOB, "blob size (%zu) exceeds maximum (%d)", blobs[i].length, maxlen); return ERR_BLOB_TOO_LARGE; } } diff --git a/db/sqlglue.c b/db/sqlglue.c index f9737bd3ed..e1575b1c41 100644 --- a/db/sqlglue.c +++ b/db/sqlglue.c @@ -1163,14 +1163,22 @@ int mem_to_ondisk(void *outbuf, struct field *f, struct mem_info *info, } } - if ((f->type == SERVER_BLOB || f->type == SERVER_BLOB2 || - f->type == SERVER_VUTF8) && - m->n > MAXBLOBLENGTH) { - rc = -1; - if (fail_reason) { - fail_reason->reason = CONVERT_FAILED_BLOB_SIZE; + /* Enforce the blob/vutf8 length limit on the *uncompressed* sqlite value, + * here, before it is packed and compressed. This MUST use the per-table + * limit (max_blob_length): an odh1 table can only store 28 bits of length, + * and a >256MB blob that compresses small would otherwise pass a downstream + * check_blob_sizes() (which sees the already-compressed size) and then be + * silently truncated into the odh1 header. When the caller did not supply + * a per-table limit, fall back to the coarse MAXBLOBLENGTH2 upper bound. */ + if (f->type == SERVER_BLOB || f->type == SERVER_BLOB2 || f->type == SERVER_VUTF8) { + unsigned int bloblimit = info->max_blob_length ? info->max_blob_length : MAXBLOBLENGTH2; + if ((unsigned int)m->n > bloblimit) { + rc = -1; + if (fail_reason) { + fail_reason->reason = CONVERT_FAILED_BLOB_SIZE; + } + return rc; } - return rc; } if (m->flags & MEM_Master) { @@ -1501,7 +1509,7 @@ int sqlite_to_ondisk(struct schema *s, const void *inp, int len, void *outp, int clen = 0; /* converted sofar */ int nblobs = 0; - struct mem_info info; + struct mem_info info = {0}; struct field_conv_opts_tz convopts = {.flags = 0}; info.s = s; @@ -9495,7 +9503,7 @@ char *sqlite3BtreeGetTblName(BtCursor *pCur) int sqlite3MakeRecordForComdb2(BtCursor *pCur, Mem *head, int nf, int *optimized) { struct sql_thread *thd = pCur->thd; - struct mem_info info; + struct mem_info info = {0}; struct field_conv_opts_tz convopts = {.flags = 0}; int nblobs = 0; int rc = 0; @@ -9518,6 +9526,9 @@ int sqlite3MakeRecordForComdb2(BtCursor *pCur, Mem *head, int nf, int *optimized info.convopts = &convopts; info.outblob = pCur->wr_blob_buffers; info.maxblobs = MAXBLOBS; + /* Enforce this table's real blob limit (256MB for odh1, up to ~2GB for + * odh2) on the uncompressed value in mem_to_ondisk(), before compression. */ + info.max_blob_length = max_blob_length_for_table(pCur->db); memset(info.outblob, 0, sizeof(blob_buffer_t) * MAXBLOBS); init_convert_failure_reason(info.fail_reason); @@ -12984,7 +12995,7 @@ int indexes_expressions_data(const struct dbtable *tbl, struct schema *sc, Mem mout = {{0}}; int nblobs = 0; struct field_conv_opts_tz convopts = {.flags = 0}; - struct mem_info info; + struct mem_info info = {0}; strbuf *sql; int i, rc; int exist = 0; diff --git a/db/sqlglue.h b/db/sqlglue.h index c05c21bfa7..869206c6af 100644 --- a/db/sqlglue.h +++ b/db/sqlglue.h @@ -32,6 +32,10 @@ struct mem_info { int maxblobs; struct convert_failure *fail_reason; int fldidx; + /* Per-table maximum blob/vutf8 length, checked against the *uncompressed* + * sqlite value in mem_to_ondisk() before it is packed/compressed. 0 means + * "unknown" -- fall back to the coarse MAXBLOBLENGTH2 upper bound. */ + unsigned int max_blob_length; }; typedef struct { diff --git a/db/toblock.c b/db/toblock.c index 14ef5bdf5b..8e8a50f375 100644 --- a/db/toblock.c +++ b/db/toblock.c @@ -4367,10 +4367,10 @@ static int toblock_main_int(struct javasp_trans_state *javasp_trans_handle, stru } else { blob_buffer_t *blob = &blobs[qblob.blobno]; if (!blob->exists) { - if (qblob.length > MAXBLOBLENGTH) { - reqerrstr(iq, COMDB2_BLOB_RC_RCV_TOO_LARGE, - "blob %d too large (%u > max size %u)", - qblob.blobno, qblob.length, MAXBLOBLENGTH); + unsigned int maxbloblen = max_blob_length_for_table(iq->usedb); + if (qblob.length > (unsigned)maxbloblen) { + reqerrstr(iq, COMDB2_BLOB_RC_RCV_TOO_LARGE, "blob %d too large (%u > max size %d)", + qblob.blobno, qblob.length, maxbloblen); rc = ERR_BLOB_TOO_LARGE; GOTOBACKOUT; } diff --git a/schemachange/sc_records.c b/schemachange/sc_records.c index afac78b987..a1d9d750cb 100644 --- a/schemachange/sc_records.c +++ b/schemachange/sc_records.c @@ -2222,7 +2222,7 @@ static int reconstruct_blob_records(struct convert_record_data *data, int blbix = 0; if (!data->blb_buf) { - data->blb_buf = malloc(MAXBLOBLENGTH + ODH_SIZE); + data->blb_buf = malloc(max_blob_length_for_table(data->from) + ODH_SIZE_RESERVE); if (!data->blb_buf) { logmsg(LOGMSG_ERROR, "%s:%d failed to malloc blob buffer\n", __func__, __LINE__); @@ -2276,9 +2276,9 @@ static int reconstruct_blob_records(struct convert_record_data *data, } /* Reconstruct the add. */ - if ((rc = bdb_reconstruct_add( - bdb_state, &rec->lsn, NULL, sizeof(genid_t), data->blb_buf, - MAXBLOBLENGTH + ODH_SIZE, &dtalen, &ixlen)) != 0) { + if ((rc = bdb_reconstruct_add(bdb_state, &rec->lsn, NULL, sizeof(genid_t), data->blb_buf, + max_blob_length_for_table(data->from) + ODH_SIZE_RESERVE, &dtalen, &ixlen)) != + 0) { logmsg(LOGMSG_ERROR, "%s:%d failed to reconstruct add rc=%d\n", __func__, __LINE__, rc); goto error; @@ -2334,7 +2334,7 @@ static int reconstruct_blob_records(struct convert_record_data *data, case DB_llog_undo_upd_dta: case DB_llog_undo_upd_dta_lk: if (!data->old_blb_buf) { - data->old_blb_buf = malloc(MAXBLOBLENGTH + ODH_SIZE); + data->old_blb_buf = malloc(max_blob_length_for_table(data->from) + ODH_SIZE_RESERVE); if (!data->old_blb_buf) { logmsg(LOGMSG_ERROR, "%s:%d failed to malloc blob buffer\n", __func__, __LINE__); @@ -2371,7 +2371,7 @@ static int reconstruct_blob_records(struct convert_record_data *data, bdb_state, &rec->lsn, data->old_blb_buf, &prevlen, data->blb_buf, &updlen, NULL, NULL, NULL); } else { - prevlen = updlen = MAXBLOBLENGTH + ODH_SIZE; + prevlen = updlen = max_blob_length_for_table(data->from) + ODH_SIZE_RESERVE; rc = bdb_reconstruct_update(bdb_state, &rec->lsn, &page, &index, NULL, NULL, data->old_blb_buf, &prevlen, NULL, NULL, data->blb_buf, diff --git a/sqlite/ext/comdb2/limits.c b/sqlite/ext/comdb2/limits.c index d2e5bd117f..da80f967d5 100644 --- a/sqlite/ext/comdb2/limits.c +++ b/sqlite/ext/comdb2/limits.c @@ -34,7 +34,7 @@ struct limit_t { } limits[] = { {"max_blob_fields", "Maximum number of blob/vutf8 fields per table", MAXBLOBS}, - {"max_blob_length", "Maximum blob length", MAXBLOBLENGTH}, + {"max_blob_length", "Maximum blob length (odh2 tables)", MAXBLOBLENGTH2}, {"max_bounded_parameters", "Maximum number of bounded parameters per prepared statement", MAXDYNTAGCOLUMNS}, diff --git a/sqlite/ext/comdb2/logicalops.c b/sqlite/ext/comdb2/logicalops.c index e1efa5283f..ed916b9799 100644 --- a/sqlite/ext/comdb2/logicalops.c +++ b/sqlite/ext/comdb2/logicalops.c @@ -534,7 +534,15 @@ static int produce_update_data_record(logicalops_cursor *pCur, DB_LOGC *logc, pCur->table = strdup((char *)(upd_dta->table.data)); } - assert(dtalen <= PACKED_MEMORY_SIZE); + /* logicalops uses a fixed 256MB scratch buffer; records larger than that + * (only possible on odh2 tables) are not yet supported here -- error rather + * than overflow. */ + if (dtalen > PACKED_MEMORY_SIZE) { + logmsg(LOGMSG_ERROR, "%s: record too large for logicalops (%d > %d)\n", + __func__, dtalen, PACKED_MEMORY_SIZE); + rc = SQLITE_INTERNAL; + goto done; + } ASSERT_PARAMETER(dtalen); genid_format(pCur, genid, pCur->genid, sizeof(pCur->genid)); genid_format(pCur, oldgenid, pCur->oldgenid, sizeof(pCur->oldgenid)); @@ -813,7 +821,12 @@ static int produce_delete_data_record(logicalops_cursor *pCur, DB_LOGC *logc, pCur->table = strdup((char *)(del_dta->table.data)); } - assert(dtalen <= PACKED_MEMORY_SIZE); + if (dtalen > PACKED_MEMORY_SIZE) { + logmsg(LOGMSG_ERROR, "%s: record too large for logicalops (%d > %d)\n", + __func__, dtalen, PACKED_MEMORY_SIZE); + rc = SQLITE_INTERNAL; + goto done; + } genid_format(pCur, genid, pCur->oldgenid, sizeof(pCur->oldgenid)); if (dtafile == 0) { diff --git a/sqlite/src/sqliteLimit.h b/sqlite/src/sqliteLimit.h index 8eb75dc7da..428c3b595b 100644 --- a/sqlite/src/sqliteLimit.h +++ b/sqlite/src/sqliteLimit.h @@ -21,7 +21,13 @@ ** to count the size: 2^31-1 or 2147483647. */ #ifndef SQLITE_MAX_LENGTH -# define SQLITE_MAX_LENGTH 1000000000 +/* Raised above the stock 1e9 so odh2 tables can round-trip large (~1GB) + * blobs/strings through SQL. Capped at INT_MAX/2 rather than the 2^31-1 hard + * limit because a value is delivered inside one newsql message whose length is + * a *signed* 32-bit field; a value near INT_MAX plus protobuf/query framing + * would overflow it and could not be sent. INT_MAX/2 leaves ample headroom. + * Supporting truly ~2GB values would require widening the wire length field. */ +# define SQLITE_MAX_LENGTH 1073741823 #endif /* diff --git a/tests/comdb2sys.test/comdb2sys.expected b/tests/comdb2sys.test/comdb2sys.expected index 20529723ac..3d9a4be569 100644 --- a/tests/comdb2sys.test/comdb2sys.expected +++ b/tests/comdb2sys.test/comdb2sys.expected @@ -317,7 +317,7 @@ (name='ZLIB', reserved='N') [SELECT * FROM comdb2_keywords WHERE reserved = 'N' ORDER BY name] rc 0 (name='max_blob_fields', description='Maximum number of blob/vutf8 fields per table', value=15) -(name='max_blob_length', description='Maximum blob length', value=268435455) +(name='max_blob_length', description='Maximum blob length (odh2 tables)', value=2147483647) (name='max_bounded_parameters', description='Maximum number of bounded parameters per prepared statement', value=2048) (name='max_column_name_length', description='Maximum column name length', value=99) (name='max_columns', description='Maximum columns per table', value=1024) diff --git a/tests/tools/comdb2_blobtest.c b/tests/tools/comdb2_blobtest.c index 39addcfae7..724d0e9179 100644 --- a/tests/tools/comdb2_blobtest.c +++ b/tests/tools/comdb2_blobtest.c @@ -4,17 +4,30 @@ #include #include #include +#include +#include #include int main(int argc, char *argv[]) { + if (argc != 4) { + printf("Usage: dbname id size\n"); + return 1; + } + char *dbname = argv[1]; - int64_t id = atoi(argv[2]); - off_t sz = atoi(argv[3]); + int64_t id = strtoll(argv[2], NULL, 0); + /* Parse the size as 64-bit: the test drives sizes at and beyond the odh2 + * ceiling (INT_MAX), which overflow atoi()/int and previously produced a + * bogus (often negative) length and a crash. */ + unsigned long long sz = strtoull(argv[3], NULL, 0); void *buf; - if (argc != 4) { - printf("Usage: dbname id size\n"); + /* cdb2_bind_param carries the blob length in an int and the newsql wire + * length is a signed int, so a blob larger than INT_MAX cannot round-trip. + * Report and exit cleanly rather than truncate the length or over-allocate. */ + if (sz > (unsigned long long)INT_MAX) { + printf("size %llu exceeds the maximum sendable blob (%d)\n", sz, INT_MAX); return 1; } @@ -23,9 +36,9 @@ int main(int argc, char *argv[]) { if (config) cdb2_set_comdb2db_config(config); - buf = calloc(1, sz); + buf = calloc(1, sz ? (size_t)sz : 1); if (buf == NULL) { - printf("can't allocate %zd bytes for buffer\n", (size_t) sz); + printf("can't allocate %llu bytes for buffer\n", sz); return 1; } cdb2_hndl_tp *db; @@ -36,7 +49,7 @@ int main(int argc, char *argv[]) { } cdb2_bind_param(db, "a", CDB2_INTEGER, &id, sizeof(int64_t)); - cdb2_bind_param(db, "b", CDB2_BLOB, buf, sz); + cdb2_bind_param(db, "b", CDB2_BLOB, buf, (int)sz); rc = cdb2_run_statement(db, "insert into t(a, b) values(@a, @b)"); if (rc) { printf("insert %d %s\n", rc, cdb2_errstr(db)); @@ -73,7 +86,7 @@ int main(int argc, char *argv[]) { cdb2_clearbindings(db); cdb2_bind_param(db, "a", CDB2_INTEGER, &id, sizeof(int64_t)); - cdb2_bind_param(db, "b", CDB2_BLOB, buf, sz); + cdb2_bind_param(db, "b", CDB2_BLOB, buf, (int)sz); rc = cdb2_run_statement(db, "update t set b=@b where a=@a"); if (rc) { printf("run b %d %s\n", rc, cdb2_errstr(db)); @@ -95,5 +108,6 @@ int main(int argc, char *argv[]) { } cdb2_close(db); + free(buf); return 0; } From 707158acb9f1de3a269015fb6a8a2049fe62b7b7 Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Fri, 31 Jul 2026 14:21:15 -0400 Subject: [PATCH 08/13] odh2: add randomize_odh2 tunable to fuzz odh1/odh2 coexistence Nothing today exercises both header formats living in the same table -- the per-record flag decode in read_odh() that most needs coverage, and the shape produced in the field when an odh1/time-based database is converted to genid48. A database is otherwise uniform per creation: genid48 forces odh2 everywhere, time-based-genid tables are odh1. Add a general, default-off tunable "randomize_odh2". When enabled, for any record that would otherwise be odh1, init_odh() flips a coin and writes odh2 instead. The random branch is only reachable when genids are time-based (where odh1 is legal), so the genid48-never-odh1 invariant is untouched -- genid48 databases still force odh2 as before. With the tunable on and a database created under time-based genids (init_with_time_based_genids + dont_init_with_odh2), every table ends up with a random mix of both formats. The coin is flipped per write, so a record's format can change from one update to the next. That is intentional and harmless: reads decode each record from its own flag byte, and it broadens coverage to both the odh1->odh2 and odh2->odh1 pack transitions. (An earlier draft enforced upgrade-only/never- downgrade, which required threading the old record's format through the write path; the plain coin flip is simpler and a better fuzzer.) No existing test selects the new comdb2_insert_timestamp / comdb2_update_timestamp columns on a base table, so the randomization is invisible to expected output and the suite passes as-is. A read-only "odh2_random_upgrades" counter (visible via the comdb2_tunables system table) lets a run confirm coexistence occurred. To run the suite as a coexistence fuzzer, point CUSTOMLRLPATH at a fragment containing: init_with_time_based_genids dont_init_with_odh2 randomize_odh2 1 and run `make -kjN CUSTOMLRLPATH=` from tests/. Normal operation is unaffected (tunable defaults off); not for production use. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/odh.c | 30 +++++++++++++++---- db/comdb2.c | 9 ++++++ db/comdb2.h | 2 ++ db/db_tunables.h | 10 +++++++ tests/tunables.test/t00_all_tunables.expected | 2 ++ 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/bdb/odh.c b/bdb/odh.c index 36b96cfe4a..b4bff61041 100644 --- a/bdb/odh.c +++ b/bdb/odh.c @@ -294,6 +294,11 @@ static void read_odh(const void *buf, struct odh *odh) odh->update_secs = 0; } +/* Test/debug coexistence fuzzer -- see db/comdb2.c and the "randomize_odh2" + * tunable. Defined in the db layer; referenced here to gate init_odh(). */ +extern int gbl_randomize_odh2; +extern int gbl_odh2_random_upgrades; + void init_odh(bdb_state_type *bdb_state, struct odh *odh, void *rec, size_t reclen, int is_blob) { @@ -324,11 +329,26 @@ void init_odh(bdb_state_type *bdb_state, struct odh *odh, void *rec, * insert. On an *update* the caller must overwrite insert_secs with the * record's original insert time so it is not reset (update_secs stays now). */ - if (bdb_state->ondisk_header && (bdb_state->odh2 || !genid_contains_time(bdb_state))) { - uint32_t now = (uint32_t)comdb2_time_epoch(); - odh->flags |= ODH2_FLAG; - odh->insert_secs = now; - odh->update_secs = now; + if (bdb_state->ondisk_header) { + int use_odh2 = (bdb_state->odh2 || !genid_contains_time(bdb_state)); + + /* Test/debug coexistence fuzzer: for a record that would otherwise be + * odh1, flip a coin and write odh2 instead. Only reachable when genids + * are time-based (odh1 is legal there), so the genid48-never-odh1 + * invariant is untouched. This is a fresh coin per write, so a record's + * format can change across updates -- fine, since reads decode each + * record from its own flag byte. */ + if (!use_odh2 && gbl_randomize_odh2 && (rand() & 1)) { + use_odh2 = 1; + gbl_odh2_random_upgrades++; /* approximate; unlocked on purpose */ + } + + if (use_odh2) { + uint32_t now = (uint32_t)comdb2_time_epoch(); + odh->flags |= ODH2_FLAG; + odh->insert_secs = now; + odh->update_secs = now; + } } } diff --git a/db/comdb2.c b/db/comdb2.c index 83f184d1dc..1fbb63c7ca 100644 --- a/db/comdb2.c +++ b/db/comdb2.c @@ -394,6 +394,15 @@ int gbl_init_with_instant_sc = 1; * db is in genid48 format, regardless of this default (a genid48 record must * never be odh1, or its insert time would be lost). */ int gbl_init_with_odh2 = 1; +/* Test/debug only: when set, writes randomly UPGRADE records from odh1 to odh2 + * (never downgrade) while genids are time-based, so a single table ends up with + * a mix of both header formats. Off in normal operation; enable it to run the + * test suite as an odh1/odh2 coexistence fuzzer. Has no effect under genid48 + * (those writes are already forced to odh2). */ +int gbl_randomize_odh2 = 0; +/* Count of records the randomizer above emitted as odh2 (approximate -- bumped + * without locking; lets a fuzz run confirm coexistence was actually produced). */ +int gbl_odh2_random_upgrades = 0; int gbl_init_with_compr = BDB_COMPRESS_CRLE; int gbl_init_with_compr_blobs = BDB_COMPRESS_LZ4; int gbl_init_with_bthash = 0; diff --git a/db/comdb2.h b/db/comdb2.h index 3b890fbaed..fc2622a376 100644 --- a/db/comdb2.h +++ b/db/comdb2.h @@ -1846,6 +1846,8 @@ extern int gbl_init_with_queue_persistent_seq; extern int gbl_init_with_ipu; extern int gbl_init_with_instant_sc; extern int gbl_init_with_odh2; +extern int gbl_randomize_odh2; +extern int gbl_odh2_random_upgrades; extern int gbl_init_with_compr; extern int gbl_init_with_queue_compr; extern int gbl_init_with_compr_blobs; diff --git a/db/db_tunables.h b/db/db_tunables.h index fefa54839b..ede0799aaf 100644 --- a/db/db_tunables.h +++ b/db/db_tunables.h @@ -661,6 +661,16 @@ REGISTER_TUNABLE("init_with_odh2", "(insert/update timestamps, 32-bit length). Requires " "on-disk header. (Default: off)", TUNABLE_BOOLEAN, &gbl_init_with_odh2, READONLY | NOARG, NULL, NULL, NULL, NULL); +REGISTER_TUNABLE("randomize_odh2", + "Test/debug only: randomly upgrade records from odh1 to odh2 " + "(never downgrade) while genids are time-based, so a table ends " + "up with a mix of both header formats. No effect under genid48. " + "Not for production use. (Default: off)", + TUNABLE_BOOLEAN, &gbl_randomize_odh2, 0, NULL, NULL, NULL, NULL); +REGISTER_TUNABLE("odh2_random_upgrades", + "Read-only count of records emitted as odh2 by 'randomize_odh2' " + "(approximate). Lets a fuzz run confirm coexistence occurred.", + TUNABLE_INTEGER, &gbl_odh2_random_upgrades, READONLY, NULL, NULL, NULL, NULL); REGISTER_TUNABLE("init_with_ondisk_header", "Initialize tables with on-disk header. (Default: on)", TUNABLE_BOOLEAN, &gbl_init_with_odh, READONLY | NOARG, NULL, diff --git a/tests/tunables.test/t00_all_tunables.expected b/tests/tunables.test/t00_all_tunables.expected index 68ffd40381..810c9766d1 100644 --- a/tests/tunables.test/t00_all_tunables.expected +++ b/tests/tunables.test/t00_all_tunables.expected @@ -670,6 +670,7 @@ (name='num_write_retries', description='number of times to retry writes on ENOSPC', type='INTEGER', value='128', read_only='N') (name='numberkdbcaches', description='Split the cache into this many segments.', type='INTEGER', value='0', read_only='N') (name='numtimesbehind', description='', type='INTEGER', value='1000000000', read_only='N') +(name='odh2_random_upgrades', description='Read-only count of records emitted as odh2 by 'randomize_odh2' (approximate). Lets a fuzz run confirm coexistence occurred.', type='INTEGER', value='0', read_only='Y') (name='oldrangexlim', description='', type='BOOLEAN', value='OFF', read_only='Y') (name='on_del_set_null_feature', description='Enables support for ON DELETE SET NULL foreign key constraint action (Default: ON)', type='BOOLEAN', value='ON', read_only='N') (name='on_pthread_create_error', description='on_pthread_create_error', type='BOOLEAN', value='ON', read_only='N') @@ -815,6 +816,7 @@ (name='rand_udp_fails', description='Rate of drop of UDP packets (for testing).', type='INTEGER', value='0', read_only='N') (name='random_lock_release_interval', description='', type='INTEGER', value='0', read_only='Y') (name='random_rowlocks', description='Grab random, guaranteed non-conflicting rowlocks', type='BOOLEAN', value='OFF', read_only='N') +(name='randomize_odh2', description='Test/debug only: randomly upgrade records from odh1 to odh2 (never downgrade) while genids are time-based, so a table ends up with a mix of both header formats. No effect under genid48. Not for production use. (Default: off)', type='BOOLEAN', value='OFF', read_only='N') (name='rangextlim', description='', type='INTEGER', value='16', read_only='Y') (name='rcache', description='Keep a lookaside cache of root pages for B-trees. (Default: off)', type='BOOLEAN', value='OFF', read_only='Y') (name='rcache_count', description='Number of entries in root page cache.', type='INTEGER', value='257', read_only='N') From 4850d60a79bba31cea75cf192d4c44abd5238147 Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Fri, 31 Jul 2026 19:45:49 -0400 Subject: [PATCH 09/13] odh2: size the remaining scratch buffers to the odh2 limit Several paths still allocated scratch buffers sized for the odh1 header or the odh1 256MB ceiling and would truncate, reject, or overrun an odh2 record even though the write and recovery paths already handle it: - schemachange/sc_records.c: the logical-redo thread (live_sc_logical_redo_thd and the live_sc_redo_add/update helpers) sized its four packed-record scratch buffers -- and the reconstruct capacity hints handed to bdb_reconstruct_add / bdb_reconstruct_update / bdb_reconstruct_inplace_update -- as lrl + ODH_SIZE (the 7-byte odh1 header). An odh2 record carries a 16-byte header, so a full-size reconstructed record overran the buffer by up to 9 bytes, corrupting the heap; the damage only surfaced later as an abort inside mspace_free during convert_record_data_cleanup (seen by the sc_redo_step test). Size these to lrl + ODH_SIZE_RESERVE (the maximum header, == ODH2_SIZE) so either header fits, matching the blob path already fixed here. The delete path already bounds its buffer by the record's exact logged length and is unchanged. - sqlite/ext/comdb2/logicalops.c: the comdb2_logicalops systable cursor kept fixed 256MB packed/packedprev buffers and errored on bigger records. Grow them on demand to the table's max_blob_length_for_table() (256MB for odh1, up to ~2GB for odh2), tracked by new packedcap/packedprevcap fields and freed in logicalopsClose(). These use plain realloc()/free() rather than sqlite3's allocator, which caps a single allocation below the odh2 ~2GB ceiling and would otherwise fail to allocate a large odh2 record. odh1 cursors are unchanged; only odh2 tables allocate more, lazily, per active cursor. The delete path sizes to the deleted record's exact logged length. Also normalise the log rectype (normalize_rectype) before the sanity assert in unpack_logical_record -- the on-disk rectype can carry the rowlocks/utxnid variant offset, which otherwise trips the assert. - bdb/rowlocks.c: the four db_printlog-only buffers (case DB_TXN_PRINT, reached only by the standalone comdb2_db_printlog tool -- not the recovery replay path) are sized to MAXBLOBLENGTH2 so a large odh2 record isn't truncated in the log dump. This only ever allocates inside that debug tool. Known ceiling left in place: the newsql wire header length is a signed int, so a single ~2GB message still can't round-trip; the SQL-insertable limit is SQLITE_MAX_LENGTH (INT_MAX/2) and MAXBLOBLENGTH2 is INT_MAX by design. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- bdb/rowlocks.c | 17 ++--- schemachange/sc_records.c | 16 ++--- sqlite/ext/comdb2/logicalops.c | 110 ++++++++++++++++++++------------- 3 files changed, 83 insertions(+), 60 deletions(-) diff --git a/bdb/rowlocks.c b/bdb/rowlocks.c index f108b258d8..91e54b1f18 100644 --- a/bdb/rowlocks.c +++ b/bdb/rowlocks.c @@ -2222,11 +2222,14 @@ static char *opstr(db_recops op) } } +/* db_printlog-only scratch buffers (see callers under case DB_TXN_PRINT). Sized + * to the odh2 maximum so a >256MB odh2 record isn't truncated in the log dump; + * this only ever allocates inside the standalone comdb2_db_printlog tool. */ static char *printmemarg1(void) { static char *u = NULL; if (!u) - u = malloc(MAXBLOBLENGTH + 7); + u = malloc((size_t)MAXBLOBLENGTH2 + 7); return u; } @@ -2234,7 +2237,7 @@ static char *printmemarg2(void) { static char *u = NULL; if (!u) - u = malloc(MAXBLOBLENGTH + 7); + u = malloc((size_t)MAXBLOBLENGTH2 + 7); return u; } @@ -2309,9 +2312,8 @@ int handle_undo_add_dta(DB_ENV *dbenv, u_int32_t rectype, if (!llldta) llldta = printmemarg1(); - bdb_reconstruct_add(bdb_state, &lll, &lllgenid, - sizeof(unsigned long long), llldta, - MAXBLOBLENGTH + 7, &lllout, NULL); + bdb_reconstruct_add(bdb_state, &lll, &lllgenid, sizeof(unsigned long long), llldta, MAXBLOBLENGTH2, &lllout, + NULL); printf(" --genid %16llx\n", lllgenid); printf(" --dta [%d] ", lllout); @@ -2403,9 +2405,8 @@ int handle_undo_add_dta_lk(DB_ENV *dbenv, u_int32_t rectype, * db_printlog. */ if (!llldta) llldta = printmemarg1(); - bdb_reconstruct_add(bdb_state, &lll, &lllgenid, - sizeof(unsigned long long), llldta, - MAXBLOBLENGTH + 7, &lllout, NULL); + bdb_reconstruct_add(bdb_state, &lll, &lllgenid, sizeof(unsigned long long), llldta, MAXBLOBLENGTH2, &lllout, + NULL); printf(" --genid %16llx\n", lllgenid); printf(" --dta [%d] ", lllout); diff --git a/schemachange/sc_records.c b/schemachange/sc_records.c index a1d9d750cb..356485a0f0 100644 --- a/schemachange/sc_records.c +++ b/schemachange/sc_records.c @@ -2442,8 +2442,8 @@ static int unpack_and_upgrade_ondisk_record(struct convert_record_data *data, void *unpack, struct odh *odh) { int rc = 0; - if ((rc = bdb_unpack(data->from->handle, dta, *dtalen, unpack, - data->from->lrl + ODH_SIZE, odh, NULL)) != 0) { + if ((rc = bdb_unpack(data->from->handle, dta, *dtalen, unpack, data->from->lrl + ODH_SIZE_RESERVE, odh, NULL)) != + 0) { logmsg(LOGMSG_ERROR, "%s:%d error unpacking buf rc=%d\n", __func__, __LINE__, rc); return rc; @@ -2559,7 +2559,7 @@ static int live_sc_redo_add(struct convert_record_data *data, DB_LOGC *logc, llog_undo_add_dta_args *add_dta = NULL; llog_undo_add_dta_lk_args *add_dta_lk = NULL; - dtalen = data->from->lrl + ODH_SIZE; + dtalen = data->from->lrl + ODH_SIZE_RESERVE; brecs.genid = rec->genid; pbrecs = hash_find(data->blob_hash, &brecs); if (pbrecs) { @@ -2956,7 +2956,7 @@ static int live_sc_redo_update(struct convert_record_data *data, DB_LOGC *logc, } else { unsigned long long prevgenid, newgenid; int prevgenidlen, newgenidlen; - prevlen = updlen = data->from->lrl + ODH_SIZE; + prevlen = updlen = data->from->lrl + ODH_SIZE_RESERVE; prevgenidlen = newgenidlen = sizeof(unsigned long long); rc = bdb_reconstruct_update(bdb_state, &rec->lsn, &page, &index, &prevgenid, &prevgenidlen, @@ -3568,10 +3568,10 @@ void *live_sc_logical_redo_thd(struct convert_record_data *data) } listc_init(&data->redo_lsns, offsetof(struct redo_genid_lsns, linkv)); - data->dta_buf = malloc(data->from->lrl + ODH_SIZE); - data->old_dta_buf = malloc(data->from->lrl + ODH_SIZE); - data->unpack_dta_buf = malloc(data->from->lrl + ODH_SIZE); - data->unpack_old_dta_buf = malloc(data->from->lrl + ODH_SIZE); + data->dta_buf = malloc(data->from->lrl + ODH_SIZE_RESERVE); + data->old_dta_buf = malloc(data->from->lrl + ODH_SIZE_RESERVE); + data->unpack_dta_buf = malloc(data->from->lrl + ODH_SIZE_RESERVE); + data->unpack_old_dta_buf = malloc(data->from->lrl + ODH_SIZE_RESERVE); data->blb_buf = NULL; data->old_blb_buf = NULL; if (!data->dta_buf || !data->old_dta_buf || !data->unpack_dta_buf || diff --git a/sqlite/ext/comdb2/logicalops.c b/sqlite/ext/comdb2/logicalops.c index ed916b9799..7d01d74d36 100644 --- a/sqlite/ext/comdb2/logicalops.c +++ b/sqlite/ext/comdb2/logicalops.c @@ -35,8 +35,6 @@ #include "comdb2systbl.h" #include "sqliteInt.h" -/* Allocate maximum for unpacking */ -#define PACKED_MEMORY_SIZE (MAXBLOBLENGTH + 7) /* Column numbers */ #define LOGICALOPS_COLUMN_START 0 @@ -72,6 +70,8 @@ struct logicalops_cursor { void *unpackedprev; void *packed; void *unpacked; + int packedcap; /* allocated size of packed (grows on demand) */ + int packedprevcap; /* allocated size of packedprev */ strbuf *jsonrec; strbuf *oldjsonrec; struct dbtable *db; @@ -133,11 +133,11 @@ static int logicalopsClose(sqlite3_vtab_cursor *cur){ if (pCur->curLsnStr) sqlite3_free(pCur->curLsnStr); if (pCur->packed) - sqlite3_free(pCur->packed); + free(pCur->packed); if (pCur->unpacked) sqlite3_free(pCur->unpacked); if (pCur->packedprev) - sqlite3_free(pCur->packedprev); + free(pCur->packedprev); if (pCur->unpackedprev) sqlite3_free(pCur->unpackedprev); if (pCur->jsonrec) @@ -150,18 +150,42 @@ static int logicalopsClose(sqlite3_vtab_cursor *cur){ return SQLITE_OK; } -static void *retrieve_packed_memory_prev(logicalops_cursor *pCur) +/* Grow packedprev/packed to at least 'need' bytes (capped at the odh2 maximum) + * and return it; NULL on allocation failure or if 'need' exceeds MAXBLOBLENGTH2. + * Sizing to the record/table need keeps odh1 cursors at the old 256MB ceiling + * while letting odh2 records exceed it. Freed in logicalopsClose(). + * + * Uses plain realloc()/free() rather than sqlite3_realloc()/sqlite3_free(): + * sqlite's allocator caps a single allocation well below an odh2 record's + * ~2GB ceiling, so a large odh2 blob would fail to allocate here. */ +static void *retrieve_packed_memory_prev(logicalops_cursor *pCur, int need) { - if (pCur->packedprev == NULL) { - pCur->packedprev = sqlite3_malloc(PACKED_MEMORY_SIZE); + if (need < 1) + need = 1; + if (need > MAXBLOBLENGTH2) + return NULL; + if (pCur->packedprev == NULL || pCur->packedprevcap < need) { + void *p = realloc(pCur->packedprev, need); + if (p == NULL) + return NULL; + pCur->packedprev = p; + pCur->packedprevcap = need; } return pCur->packedprev; } -static void *retrieve_packed_memory(logicalops_cursor *pCur) +static void *retrieve_packed_memory(logicalops_cursor *pCur, int need) { - if (pCur->packed == NULL) { - pCur->packed = sqlite3_malloc(PACKED_MEMORY_SIZE); + if (need < 1) + need = 1; + if (need > MAXBLOBLENGTH2) + return NULL; + if (pCur->packed == NULL || pCur->packedcap < need) { + void *p = realloc(pCur->packed, need); + if (p == NULL) + return NULL; + pCur->packed = p; + pCur->packedcap = need; } return pCur->packed; } @@ -534,15 +558,6 @@ static int produce_update_data_record(logicalops_cursor *pCur, DB_LOGC *logc, pCur->table = strdup((char *)(upd_dta->table.data)); } - /* logicalops uses a fixed 256MB scratch buffer; records larger than that - * (only possible on odh2 tables) are not yet supported here -- error rather - * than overflow. */ - if (dtalen > PACKED_MEMORY_SIZE) { - logmsg(LOGMSG_ERROR, "%s: record too large for logicalops (%d > %d)\n", - __func__, dtalen, PACKED_MEMORY_SIZE); - rc = SQLITE_INTERNAL; - goto done; - } ASSERT_PARAMETER(dtalen); genid_format(pCur, genid, pCur->genid, sizeof(pCur->genid)); genid_format(pCur, oldgenid, pCur->oldgenid, sizeof(pCur->oldgenid)); @@ -551,32 +566,38 @@ static int produce_update_data_record(logicalops_cursor *pCur, DB_LOGC *logc, else snprintf(pCur->opstring, sizeof(pCur->opstring), "update-blob"); - if ((packedbuf = retrieve_packed_memory(pCur)) == NULL) { + if ((pCur->db = get_dbtable_by_name(pCur->table)) == NULL) { + logmsg(LOGMSG_ERROR, "%s line %d error finding dbtable %s\n", __func__, + __LINE__, pCur->table); + return SQLITE_INTERNAL; + } + + /* Size the scratch buffers to this table's maximum record/blob length + * (256MB for odh1, up to ~2GB for odh2) instead of a fixed 256MB, so an + * odh2 record is not truncated. Buffers grow on demand and are freed when + * the cursor closes. */ + int cap = max_blob_length_for_table(pCur->db); + if ((packedbuf = retrieve_packed_memory(pCur, cap)) == NULL) { logmsg(LOGMSG_ERROR, "%s line %d allocating memory\n", __func__, __LINE__); rc = SQLITE_NOMEM; goto done; } - if ((packedprevbuf = retrieve_packed_memory_prev(pCur)) == NULL) { + if ((packedprevbuf = retrieve_packed_memory_prev(pCur, cap)) == NULL) { logmsg(LOGMSG_ERROR, "%s line %d allocating memory\n", __func__, __LINE__); rc = SQLITE_NOMEM; goto done; } - if ((pCur->db = get_dbtable_by_name(pCur->table)) == NULL) { - logmsg(LOGMSG_ERROR, "%s line %d error finding dbtable %s\n", __func__, - __LINE__, pCur->table); - return SQLITE_INTERNAL; - } - /* Reconstruct record from berkley */ if (0 == bdb_inplace_cmp_genids(pCur->db->handle, oldgenid, genid)) { + prevlen = updlen = cap; rc = bdb_reconstruct_inplace_update(bdb_state, &rec->lsn, packedprevbuf, &prevlen, packedbuf, &updlen, NULL, NULL, NULL); } else { - prevlen = updlen = PACKED_MEMORY_SIZE; + prevlen = updlen = cap; rc = bdb_reconstruct_update(bdb_state, &rec->lsn, &page, &index, NULL, NULL, packedprevbuf, &prevlen, NULL, NULL, packedbuf, &updlen); @@ -692,7 +713,6 @@ static int produce_add_data_record(logicalops_cursor *pCur, DB_LOGC *logc, reset_record_state(pCur); - dtalen = PACKED_MEMORY_SIZE; if (rec->type == DB_llog_undo_add_dta_lk) { if ((rc = llog_undo_add_dta_lk_read(bdb_state->dbenv, logdta->data,&add_dta_lk)) != 0) { @@ -723,21 +743,24 @@ static int produce_add_data_record(logicalops_cursor *pCur, DB_LOGC *logc, snprintf(pCur->opstring, sizeof(pCur->opstring), "insert-blob"); } - if ((packedbuf = retrieve_packed_memory(pCur)) == NULL) { - logmsg(LOGMSG_ERROR, "%s line %d allocating memory\n", __func__, - __LINE__); - rc = SQLITE_NOMEM; - goto done; - } - if ((pCur->db = get_dbtable_by_name(pCur->table)) == NULL) { logmsg(LOGMSG_ERROR, "%s line %d error finding dbtable %s\n", __func__, __LINE__, pCur->table); return SQLITE_INTERNAL; } + /* Size the scratch buffer to this table's maximum record/blob length so an + * odh2 record larger than the old fixed 256MB isn't truncated on read. */ + dtalen = max_blob_length_for_table(pCur->db); + if ((packedbuf = retrieve_packed_memory(pCur, dtalen)) == NULL) { + logmsg(LOGMSG_ERROR, "%s line %d allocating memory\n", __func__, + __LINE__); + rc = SQLITE_NOMEM; + goto done; + } + /* Reconstruct record from berkley */ - if ((rc = bdb_reconstruct_add(bdb_state, &rec->lsn, + if ((rc = bdb_reconstruct_add(bdb_state, &rec->lsn, NULL, sizeof(genid_t), packedbuf, dtalen, &dtalen, &ixlen)) != 0) { logmsg(LOGMSG_ERROR, "%s line %d error %d reconstructing insert for " "%d:%d\n", __func__, __LINE__, rc, rec->lsn.file, @@ -821,12 +844,6 @@ static int produce_delete_data_record(logicalops_cursor *pCur, DB_LOGC *logc, pCur->table = strdup((char *)(del_dta->table.data)); } - if (dtalen > PACKED_MEMORY_SIZE) { - logmsg(LOGMSG_ERROR, "%s: record too large for logicalops (%d > %d)\n", - __func__, dtalen, PACKED_MEMORY_SIZE); - rc = SQLITE_INTERNAL; - goto done; - } genid_format(pCur, genid, pCur->oldgenid, sizeof(pCur->oldgenid)); if (dtafile == 0) { @@ -835,7 +852,9 @@ static int produce_delete_data_record(logicalops_cursor *pCur, DB_LOGC *logc, snprintf(pCur->opstring, sizeof(pCur->opstring), "delete-blob"); } - if ((packedprevbuf = retrieve_packed_memory_prev(pCur)) == NULL) { + /* Size the scratch buffer to the deleted record's actual length (from the + * log) so an odh2 record larger than the old fixed 256MB isn't truncated. */ + if ((packedprevbuf = retrieve_packed_memory_prev(pCur, dtalen)) == NULL) { logmsg(LOGMSG_ERROR, "%s line %d allocating memory\n", __func__, __LINE__); rc = SQLITE_NOMEM; @@ -923,6 +942,9 @@ static int unpack_logical_record(logicalops_cursor *pCur) return SQLITE_INTERNAL; } LOGCOPY_32(&rectype, logdta.data); + /* The on-disk rectype may carry the rowlocks/utxnid variant offset; + * normalise it to the base type before comparing against rec->type. */ + normalize_rectype(&rectype); assert(rectype == rec->type); switch(rec->type) { From 1ef24cecc993d52ac7a4e44c0d6786d74fc21318 Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Fri, 31 Jul 2026 19:50:06 -0400 Subject: [PATCH 10/13] odh2: test the insert/update timestamp columns New test odh2_timestamps.test: on an odh2 table (init_with_odh2, time-based genids, randomizer off) it checks that comdb2_insert_timestamp / comdb2_update_timestamp / comdb2_rowtimestamp are real datetimes, that all three match on a fresh insert, and that after an update the insert timestamp is preserved while the update timestamp advances. Assertions reduce to booleans/typeof/server-side comparisons so no raw datetime is diffed as text. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- tests/odh2_timestamps.test/Makefile | 8 +++++ tests/odh2_timestamps.test/lrl.options | 3 ++ tests/odh2_timestamps.test/runit | 46 ++++++++++++++++++++++++++ tests/odh2_timestamps.test/t.csc2 | 8 +++++ 4 files changed, 65 insertions(+) create mode 100644 tests/odh2_timestamps.test/Makefile create mode 100644 tests/odh2_timestamps.test/lrl.options create mode 100755 tests/odh2_timestamps.test/runit create mode 100644 tests/odh2_timestamps.test/t.csc2 diff --git a/tests/odh2_timestamps.test/Makefile b/tests/odh2_timestamps.test/Makefile new file mode 100644 index 0000000000..0df466c9d4 --- /dev/null +++ b/tests/odh2_timestamps.test/Makefile @@ -0,0 +1,8 @@ +ifeq ($(TESTSROOTDIR),) + include ../testcase.mk +else + include $(TESTSROOTDIR)/testcase.mk +endif +ifeq ($(TEST_TIMEOUT),) + export TEST_TIMEOUT=2m +endif diff --git a/tests/odh2_timestamps.test/lrl.options b/tests/odh2_timestamps.test/lrl.options new file mode 100644 index 0000000000..f2319a1f56 --- /dev/null +++ b/tests/odh2_timestamps.test/lrl.options @@ -0,0 +1,3 @@ +init_with_odh2 +init_with_time_based_genids +randomize_odh2 0 diff --git a/tests/odh2_timestamps.test/runit b/tests/odh2_timestamps.test/runit new file mode 100755 index 0000000000..efaff242d2 --- /dev/null +++ b/tests/odh2_timestamps.test/runit @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# Verify the odh2 insert/update timestamp columns. The table is forced to odh2 +# (init_with_odh2) under time-based genids, with the coexistence randomizer off, +# so the behavior is deterministic. Assertions reduce to booleans / typeof / +# server-side comparisons so no raw datetime is ever compared as text. +bash -n "$0" || exit 1 +source ${TESTSROOTDIR}/tools/runit_common.sh + +dbname=$1 +[[ -z "$dbname" ]] && failexit "dbname missing" + +cdb2sql ${CDB2_OPTIONS} $dbname default "create table t { `cat t.csc2` }" +cdb2sql ${CDB2_OPTIONS} $dbname default "insert into t(a, c) values(1, 1)" + +# odh2 rows expose real datetime timestamps. +r=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select typeof(comdb2_insert_timestamp) from t where a=1") +assertres "$r" "datetime" "insert_timestamp is a datetime" + +r=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select typeof(comdb2_update_timestamp) from t where a=1") +assertres "$r" "datetime" "update_timestamp is a datetime" + +# Right after insert: insert == update == rowtimestamp. +r=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select comdb2_update_timestamp = comdb2_insert_timestamp \ + and comdb2_rowtimestamp = comdb2_insert_timestamp from t where a=1") +assertres "$r" "1" "insert==update==rowtimestamp on a fresh insert" + +# Capture the insert timestamp, wait, update the row, then confirm the insert +# timestamp is preserved while the update timestamp advances. +before=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select comdb2_insert_timestamp from t where a=1") +sleep 2 +cdb2sql ${CDB2_OPTIONS} $dbname default "update t set c=100 where a=1" + +after=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select comdb2_insert_timestamp from t where a=1") +assertres "$after" "$before" "insert_timestamp preserved across an update" + +r=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select comdb2_update_timestamp > comdb2_insert_timestamp from t where a=1") +assertres "$r" "1" "update_timestamp advances after an update" + +echo "passed" +exit 0 diff --git a/tests/odh2_timestamps.test/t.csc2 b/tests/odh2_timestamps.test/t.csc2 new file mode 100644 index 0000000000..22960f416c --- /dev/null +++ b/tests/odh2_timestamps.test/t.csc2 @@ -0,0 +1,8 @@ +schema { + int a + int c null=yes +} + +keys { + "a" = a +} From ab409248f2f345772c8b7c02c6c7b5c29aca8f0c Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Fri, 31 Jul 2026 19:50:14 -0400 Subject: [PATCH 11/13] odh2: test blobs larger than the odh1 256MB ceiling New test odh2_bigblob.test: mirrors blob_size_limit.test but on an init_with_odh2 table, driving comdb2_blobtest at sizes through and beyond 256MB (up to 512MB). Where the odh1 test expects failures/zero length at >=256MB, the odh2 table stores them successfully (full length), and verify decodes every large record. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- tests/odh2_bigblob.test/Makefile | 8 ++++++ tests/odh2_bigblob.test/expected.txt | 7 +++++ tests/odh2_bigblob.test/lrl.options | 2 ++ tests/odh2_bigblob.test/runit | 38 ++++++++++++++++++++++++++++ tests/odh2_bigblob.test/t.csc2 | 8 ++++++ 5 files changed, 63 insertions(+) create mode 100644 tests/odh2_bigblob.test/Makefile create mode 100644 tests/odh2_bigblob.test/expected.txt create mode 100644 tests/odh2_bigblob.test/lrl.options create mode 100755 tests/odh2_bigblob.test/runit create mode 100644 tests/odh2_bigblob.test/t.csc2 diff --git a/tests/odh2_bigblob.test/Makefile b/tests/odh2_bigblob.test/Makefile new file mode 100644 index 0000000000..b4c0ac1057 --- /dev/null +++ b/tests/odh2_bigblob.test/Makefile @@ -0,0 +1,8 @@ +ifeq ($(TESTSROOTDIR),) + include ../testcase.mk +else + include $(TESTSROOTDIR)/testcase.mk +endif +ifeq ($(TEST_TIMEOUT),) + export TEST_TIMEOUT=5m +endif diff --git a/tests/odh2_bigblob.test/expected.txt b/tests/odh2_bigblob.test/expected.txt new file mode 100644 index 0000000000..58ea6c9722 --- /dev/null +++ b/tests/odh2_bigblob.test/expected.txt @@ -0,0 +1,7 @@ +(a=1, length(b)=1) +(a=2, length(b)=1000000) +(a=3, length(b)=100000000) +(a=4, length(b)=268435455) +(a=5, length(b)=268435456) +(a=6, length(b)=300000000) +(a=7, length(b)=536870912) diff --git a/tests/odh2_bigblob.test/lrl.options b/tests/odh2_bigblob.test/lrl.options new file mode 100644 index 0000000000..0dcda28b2b --- /dev/null +++ b/tests/odh2_bigblob.test/lrl.options @@ -0,0 +1,2 @@ +init_with_odh2 +randomize_odh2 0 diff --git a/tests/odh2_bigblob.test/runit b/tests/odh2_bigblob.test/runit new file mode 100755 index 0000000000..55b72741c1 --- /dev/null +++ b/tests/odh2_bigblob.test/runit @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# Verify that an odh2 table accepts blobs larger than the odh1 256MB ceiling. +# Mirrors blob_size_limit.test but on an init_with_odh2 table, where sizes at and +# above 268435456 (256MB) succeed instead of failing. comdb2_blobtest inserts, +# reads back, deletes, re-inserts and updates the blob for each id; on success it +# prints nothing, so out.txt should contain only the final select rows. +bash -n "$0" || exit 1 +source ${TESTSROOTDIR}/tools/runit_common.sh + +dbname=$1 +[[ -z "$dbname" ]] && failexit "dbname missing" + +cdb2sql ${CDB2_OPTIONS} $dbname default "drop table if exists t" +cdb2sql ${CDB2_OPTIONS} $dbname default "create table t { `cat t.csc2` }" + +( +${TESTSBUILDDIR}/comdb2_blobtest $dbname 1 1 +${TESTSBUILDDIR}/comdb2_blobtest $dbname 2 1000000 +${TESTSBUILDDIR}/comdb2_blobtest $dbname 3 100000000 +${TESTSBUILDDIR}/comdb2_blobtest $dbname 4 268435455 +${TESTSBUILDDIR}/comdb2_blobtest $dbname 5 268435456 +${TESTSBUILDDIR}/comdb2_blobtest $dbname 6 300000000 +${TESTSBUILDDIR}/comdb2_blobtest $dbname 7 536870912 +) > out.txt 2>&1 +cdb2sql -s ${CDB2_OPTIONS} $dbname default \ + "select a, length(b) from t order by a" >> out.txt + +if ! diff out.txt expected.txt >/dev/null ; then + echo "failed - unexpected output:" + diff expected.txt out.txt + exit 1 +fi + +# read_odh must decode every (large, odh2) record cleanly +do_verify t + +echo "passed" +exit 0 diff --git a/tests/odh2_bigblob.test/t.csc2 b/tests/odh2_bigblob.test/t.csc2 new file mode 100644 index 0000000000..32837f6cde --- /dev/null +++ b/tests/odh2_bigblob.test/t.csc2 @@ -0,0 +1,8 @@ +schema { + int a + blob b +} + +keys { + "a" = a +} From feba5889897cec3b936379ecff483e19b269602b Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Fri, 31 Jul 2026 19:50:24 -0400 Subject: [PATCH 12/13] odh2: test odh1/odh2 coexistence via genid48 conversion New test odh2_coexist.test: creates a table under time-based genids with odh2 off (odh1 rows), converts the db to genid48 with "put genid48 enable", then inserts new rows and updates some original ones so the table holds both odh1 and odh2 records at once. verify drives read_odh() over the mixed table, row counts and updated values are checked, and every row reports a non-null rowtimestamp (genid-derived for odh1, header-derived for odh2). This mirrors the real migration the feature exists for. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- tests/odh2_coexist.test/Makefile | 8 +++++ tests/odh2_coexist.test/lrl.options | 3 ++ tests/odh2_coexist.test/runit | 52 +++++++++++++++++++++++++++++ tests/odh2_coexist.test/t.csc2 | 8 +++++ 4 files changed, 71 insertions(+) create mode 100644 tests/odh2_coexist.test/Makefile create mode 100644 tests/odh2_coexist.test/lrl.options create mode 100755 tests/odh2_coexist.test/runit create mode 100644 tests/odh2_coexist.test/t.csc2 diff --git a/tests/odh2_coexist.test/Makefile b/tests/odh2_coexist.test/Makefile new file mode 100644 index 0000000000..b4c0ac1057 --- /dev/null +++ b/tests/odh2_coexist.test/Makefile @@ -0,0 +1,8 @@ +ifeq ($(TESTSROOTDIR),) + include ../testcase.mk +else + include $(TESTSROOTDIR)/testcase.mk +endif +ifeq ($(TEST_TIMEOUT),) + export TEST_TIMEOUT=5m +endif diff --git a/tests/odh2_coexist.test/lrl.options b/tests/odh2_coexist.test/lrl.options new file mode 100644 index 0000000000..59572a76f1 --- /dev/null +++ b/tests/odh2_coexist.test/lrl.options @@ -0,0 +1,3 @@ +init_with_time_based_genids +dont_init_with_odh2 +randomize_odh2 0 diff --git a/tests/odh2_coexist.test/runit b/tests/odh2_coexist.test/runit new file mode 100755 index 0000000000..faa134b0f5 --- /dev/null +++ b/tests/odh2_coexist.test/runit @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# Verify odh1 and odh2 records coexist in one table and are read back correctly. +# The db starts with time-based genids and odh2 off, so early rows are odh1. We +# then convert the db to genid48 (the real migration path), which forces every +# subsequent write to odh2: new inserts are odh2, and updating an original row +# upgrades it from odh1 to odh2. The randomizer is off so the split is +# deterministic. +bash -n "$0" || exit 1 +source ${TESTSROOTDIR}/tools/runit_common.sh + +dbname=$1 +[[ -z "$dbname" ]] && failexit "dbname missing" + +master=$(getmaster) +[[ -z "$master" ]] && failexit "could not find master" + +cdb2sql ${CDB2_OPTIONS} $dbname default "create table t { `cat t.csc2` }" + +# Phase 1: odh1 rows (time-based genids, odh2 attr off). +cdb2sql ${CDB2_OPTIONS} $dbname default \ + "insert into t(a, c) select value, value from generate_series(1, 100)" +assertcnt t 100 + +# Convert the db to genid48; subsequent writes must be odh2. +cdb2sql ${CDB2_OPTIONS} $dbname --host $master "put genid48 enable" +sleep 2 + +# Phase 2: new odh2 rows, plus updates that upgrade some original odh1 rows. +cdb2sql ${CDB2_OPTIONS} $dbname default \ + "insert into t(a, c) select value, value from generate_series(101, 200)" +cdb2sql ${CDB2_OPTIONS} $dbname default "update t set c = c + 1000 where a <= 50" + +# The table now holds odh1 rows (a=51..100, untouched) and odh2 rows +# (a=1..50 upgraded, a=101..200 fresh). Everything must read back. +assertcnt t 200 + +# verify drives read_odh() over every record regardless of header format. +do_verify t + +# Every row must report a sane (non-null) rowtimestamp: odh1 rows from the +# time-based genid, odh2 rows from the header. +nulls=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select count(*) from t where comdb2_rowtimestamp is null") +assertres "$nulls" "0" "no null rowtimestamps across mixed formats" + +# The updated rows kept their values. +updated=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select count(*) from t where a <= 50 and c = a + 1000") +assertres "$updated" "50" "updated (upgraded) rows kept their new values" + +echo "passed" +exit 0 diff --git a/tests/odh2_coexist.test/t.csc2 b/tests/odh2_coexist.test/t.csc2 new file mode 100644 index 0000000000..22960f416c --- /dev/null +++ b/tests/odh2_coexist.test/t.csc2 @@ -0,0 +1,8 @@ +schema { + int a + int c null=yes +} + +keys { + "a" = a +} From 214f265502ff120bf4ae1de260dbefbcf63be1a8 Mon Sep 17 00:00:00 2001 From: Mark Hannum Date: Fri, 31 Jul 2026 19:50:24 -0400 Subject: [PATCH 13/13] odh2: test the randomize_odh2 coexistence fuzzer New test odh2_randomize.test: with randomize_odh2 on under time-based genids, inserts many rows so the per-record coin produces both formats, asserts the odh2_random_upgrades counter (via comdb2_tunables) is greater than zero, then verifies the mixed table and checks all rowtimestamps are non-null. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Mark Hannum --- tests/odh2_randomize.test/Makefile | 8 ++++++ tests/odh2_randomize.test/lrl.options | 3 ++ tests/odh2_randomize.test/runit | 40 +++++++++++++++++++++++++++ tests/odh2_randomize.test/t.csc2 | 7 +++++ 4 files changed, 58 insertions(+) create mode 100644 tests/odh2_randomize.test/Makefile create mode 100644 tests/odh2_randomize.test/lrl.options create mode 100755 tests/odh2_randomize.test/runit create mode 100644 tests/odh2_randomize.test/t.csc2 diff --git a/tests/odh2_randomize.test/Makefile b/tests/odh2_randomize.test/Makefile new file mode 100644 index 0000000000..2d39e80074 --- /dev/null +++ b/tests/odh2_randomize.test/Makefile @@ -0,0 +1,8 @@ +ifeq ($(TESTSROOTDIR),) + include ../testcase.mk +else + include $(TESTSROOTDIR)/testcase.mk +endif +ifeq ($(TEST_TIMEOUT),) + export TEST_TIMEOUT=3m +endif diff --git a/tests/odh2_randomize.test/lrl.options b/tests/odh2_randomize.test/lrl.options new file mode 100644 index 0000000000..1b436bdb0f --- /dev/null +++ b/tests/odh2_randomize.test/lrl.options @@ -0,0 +1,3 @@ +init_with_time_based_genids +dont_init_with_odh2 +randomize_odh2 1 diff --git a/tests/odh2_randomize.test/runit b/tests/odh2_randomize.test/runit new file mode 100755 index 0000000000..aa58641a92 --- /dev/null +++ b/tests/odh2_randomize.test/runit @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Verify the randomize_odh2 coexistence fuzzer: under time-based genids with the +# tunable on, individual records are randomly written as odh1 or odh2, so a +# single table ends up with a mix. We insert enough rows that the coin lands on +# odh2 at least once (odh2_random_upgrades > 0), then confirm every record reads +# back and verifies. +bash -n "$0" || exit 1 +source ${TESTSROOTDIR}/tools/runit_common.sh + +dbname=$1 +[[ -z "$dbname" ]] && failexit "dbname missing" + +cdb2sql ${CDB2_OPTIONS} $dbname default "create table t { `cat t.csc2` }" + +# Insert plenty of rows so the randomizer produces both formats. +cdb2sql ${CDB2_OPTIONS} $dbname default \ + "insert into t(a) select value from generate_series(1, 2000)" +assertcnt t 2000 + +# The randomizer counter must show that some records were emitted as odh2. The +# randomizer runs in init_odh on the master only (replicants apply already-packed +# records), so read the counter from the master rather than via 'default', which +# in a cluster can land on a replicant that never ran the randomizer. +master=$(getmaster) +[[ -z "$master" ]] && failexit "could not find master" +upg=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname --host $master \ + "select value from comdb2_tunables where name='odh2_random_upgrades'") +if ! [[ "$upg" =~ ^[0-9]+$ ]] || [[ "$upg" -le 0 ]] ; then + failexit "expected odh2_random_upgrades > 0, got '$upg'" +fi + +# Mixed odh1/odh2 records must all decode. +do_verify t + +nulls=$(cdb2sql --tabs ${CDB2_OPTIONS} $dbname default \ + "select count(*) from t where comdb2_rowtimestamp is null") +assertres "$nulls" "0" "no null rowtimestamps across randomized formats" + +echo "passed" +exit 0 diff --git a/tests/odh2_randomize.test/t.csc2 b/tests/odh2_randomize.test/t.csc2 new file mode 100644 index 0000000000..3f347d5c72 --- /dev/null +++ b/tests/odh2_randomize.test/t.csc2 @@ -0,0 +1,7 @@ +schema { + int a +} + +keys { + "a" = a +}