cpp-database-sindex-ready-state

Answer

Root cause 1 — sindex_config_t reconstructed from disk lost BRIN fields, so operator==(goal, current) never matched.

store_t::sindex_list() rebuilt each sindex_config_t from the persisted metadata but copied only name/expr. The BRIN fields (brin, brin_columns, brin_range_size) stayed at their defaults, so the manager pump compared goal (with BRIN set, e.g. {"brin":true,"brin_columns":["sku"],"brin_range_size":512}) against current (all-default) and dropped + recreated the index on every tick — ready never flipped to true.

The config type and its equality (unchanged, shown for context):

struct sindex_config_t {
    std::string name;
    std::string expr;                          // value expression
    bool brin = false;                         // block-range index mode
    std::vector<std::string> brin_columns;     // columns for BRIN range map
    uint32_t brin_range_size = 0;              // blocks per BRIN range
    bool operator==(const sindex_config_t& o) const {
        return name == o.name && expr == o.expr &&
               brin == o.brin &&
               brin_columns == o.brin_columns &&
               brin_range_size == o.brin_range_size;
    }
};

The fix — copy the three BRIN fields when reconstructing from disk:

std::vector<sindex_config_t> store_t::sindex_list() const {
    std::vector<sindex_config_t> out;
    for (const sindex_meta_t& m : meta_.scan(KEY_SINDEX_META)) {
        sindex_config_t cfg;
        cfg.name = m.name;
        cfg.expr = m.expr;
        // BUGFIX: preserve BRIN settings so the manager's
        // goal==current comparison can actually converge.
        cfg.brin            = m.brin;            // 1
        cfg.brin_columns    = m.brin_columns;    // 2
        cfg.brin_range_size = m.brin_range_size; // 3
        out.push_back(std::move(cfg));
    }
    return out;
}

With that, the manager pump converges and marks the index ready instead of looping drop→create:

void sindex_manager_t::pump(store_t& store) {
    for (const auto& goal : store.sindex_goal_list()) {
        auto it = std::find_if(store.sindex_list().begin(), store.sindex_list().end(),
                               [&](const auto& c) { return c.name == goal.name; });
        bool ready = it != store.sindex_list().end() && (*it == goal);   // now matches for BRIN
        if (!ready) { store.drop_sindex(goal.name); store.create_sindex(goal); }
    }
}

Root cause 2 — NULL_BLOCK_ID sidecar returned an empty stream (silent data loss) instead of a full-scan fallback.

BRIN lookup resolves a key to a block, then reads the sidecar stream for that block. When the block id came back as NULL_BLOCK_ID (block freed, mapping not yet flushed, or sidecar record missing), the code returned an empty stream — callers treated it as "zero matching rows," silently dropping data that a full scan would have found.

The fix — never return an empty stream for NULL_BLOCK_ID; fall back to a full scan and recheck the block mapping so only live rows are emitted:

// BUGGY: silent data loss
sindex_stream_t brin_sidecar_t::read(block_id_t bid) {
    if (bid == NULL_BLOCK_ID) return {};   // caller sees "no rows" — WRONG
    return sidecar_.get(bid);
}

// FIXED: NULL_BLOCK_ID and missing sidecar records fall back to full scan
sindex_stream_t brin_sidecar_t::read(block_id_t bid) const {
    if (bid == NULL_BLOCK_ID || !sidecar_.has(bid)) {
        return full_scan_fallback();       // rebuild mapping from base data
    }
    return sidecar_.get(bid);
}

sindex_stream_t brin_sidecar_t::full_scan_fallback() const {
    sindex_stream_t out;
    for (const block_handle_t& blk : store_.blocks()) {
        if (blk.id == NULL_BLOCK_ID) continue;             // tombstoned/freed
        // Recheck mapping: re-derive the BRIN range for this block from the
        // actual keys so only rows that genuinely satisfy the range map are
        // emitted, then union the range's key stream.
        if (matches_brin_range(blk, brin_columns_, brin_range_size_))
            out.append(blk.key_stream());
    }
    return out;
}