diff --git a/CHANGELOG.md b/CHANGELOG.md index de1b9b7..ac78634 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,16 @@ unreleased. For the forward-looking plan see - Lost delete marks under concurrent same-chunk-group deletes. - Relation-reference leak in parallel `CREATE INDEX`. +### Removed + +- The decompressed-chunk cache, and the `pgcolumnar.enable_column_cache` and + `pgcolumnar.column_cache_size` settings with it. Its only entry point had lost + its caller when the earlier on-disk format was removed, so the cache had done + nothing since. Two settings and four passages of documentation described a + feature that did not run. A `postgresql.conf` that sets either parameter must + drop the line. The implementation is in the git history if the performance case + is made again against the current reader. + ### Changed - FSST string encoding is now kept only when it reduces the compressed chunk by diff --git a/Makefile b/Makefile index d82bfe8..9e1ffa0 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,6 @@ OBJS = \ src/columnar_reader.o \ src/columnar_delete_vector.o \ src/columnar_customscan.o \ - src/columnar_cache.o \ src/columnar_vector.o \ src/columnar_vacuum.o \ src/columnar_unique.o \ diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b1cdd00..224e83a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -75,7 +75,7 @@ storage, bulk and single insert, sequential scan open/next/close, delete and update (through the row mask), fetch a row by item pointer, size estimation, and truncate. It also holds `_PG_init`, which registers every `pgcolumnar.*` GUC (the compression codec and level, the row-group and vector row limits, and the qual -pushdown, custom scan, vectorized aggregate, and column cache toggles), the +pushdown, custom scan and vectorized aggregate toggles), the pre-commit hook that flushes pending writes, and the object-access hook that removes a table's metadata rows when the table is dropped. @@ -208,15 +208,6 @@ answered from the zone-map metadata, falling back to a scan-and-fold when the group has deletes. It is chosen only when every aggregate, column type, and clause is supported; anything else falls back to the scalar plan. -### columnar_cache.c -The optional decompressed-chunk cache, off by default behind -`pgcolumnar.enable_column_cache` and bounded by `pgcolumnar.column_cache_size` -megabytes. It is a backend-local, LRU-bounded cache of decompressed value -streams keyed by storage id and absolute logical offset. It returns a fresh copy -to the caller so eviction is always safe, and it is flushed on any relcache -invalidation so a truncate offset reuse or a vacuum storage swap can never serve -a stale buffer. It only avoids repeated decompression; it never changes results. - ### columnar_vacuum.c Compaction, statistics, and storage-id lookup. `pgcolumnar.vacuum` materializes a relation's live rows (the reader skips row-mask-deleted rows), swaps the @@ -373,7 +364,7 @@ Scan: 2. The reader (`columnar_reader`) goes through the row groups. It uses the zone maps in `columnar_metadata` to skip groups and vectors. 3. The reader decodes the projected chunks through `columnar_encoding` and - `columnar_compression`, and can use `columnar_cache`. + `columnar_compression`. 4. The reader applies the delete vector (`columnar_delete_vector`) and returns the rows one at a time. 5. The executor applies the full qual again, as a filter. diff --git a/docs/administration.md b/docs/administration.md index 2485760..99de2f8 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -205,11 +205,9 @@ specific reason to change it. ## Column cache -`pgcolumnar.enable_column_cache` keeps chunk groups after decompression. Other -reads can then use them again. `pgcolumnar.column_cache_size` sets the size, and -the default is 200 MB. The cache is off by default. Enable it if you scan the -same recent data more than one time. Set the size to the size of the working -set. +There is no cache of decompressed chunk groups. A cache existed in an earlier +build, but its only entry point lost its caller and the code did nothing. It was +removed in #303 rather than left as a setting that changes nothing. ## Backup and restore diff --git a/docs/configuration.md b/docs/configuration.md index a5f83ed..ace376c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -69,8 +69,6 @@ disk. It never changes the values that a table returns. | Setting | Type | Default | Description | | --- | --- | --- | --- | -| `pgcolumnar.enable_column_cache` | boolean | `off` | Cache decompressed chunk groups so they can be reused across reads. | -| `pgcolumnar.column_cache_size` | integer (MB) | `200` | Size of the decompressed-chunk cache. Applies when the column cache is enabled. Range 1 to INT_MAX. | ### Maintenance and disk reclaim diff --git a/docs/testing.md b/docs/testing.md index 32cc752..03986c3 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -10,7 +10,7 @@ test/phase2.sh /path/to/pg_config # compression, projection, min/max skip, fi test/phase3.sh /path/to/pg_config # delete, update, MVCC, savepoints, temp tables test/phase4.sh /path/to/pg_config # btree/hash indexes, constraints, conversion test/phase5.sh /path/to/pg_config # custom scan, pushdown, options, vacuum -test/phase6.sh /path/to/pg_config # aggregate correctness and the column cache +test/phase6.sh /path/to/pg_config # aggregate correctness test/audit.sh /path/to/pg_config # regression tests for audited defects test/concurrency.sh /path/to/pg_config # concurrent same-chunk-group deletes test/unique_conc.sh /path/to/pg_config # concurrent same-unique-key inserts diff --git a/src/columnar.h b/src/columnar.h index 106a832..3b9e4e3 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -173,11 +173,9 @@ extern bool columnar_enable_bloom_filter; /* bloom equality skipping (I7) */ /* Phase 6 GUCs (spec 8.3) */ extern bool columnar_enable_vectorization; /* vectorized aggregate path */ -extern bool columnar_enable_column_cache; /* decompressed-chunk cache */ extern bool columnar_enable_read_stream; /* stream/prefetch block reads (PG17+) */ extern bool columnar_enable_index_only_scan; /* allow index-only scans (gap 28) */ extern bool columnar_enable_projection_scan; /* scan a covering projection (gap 26) */ -extern int columnar_column_cache_size; /* cache budget in megabytes */ /* issue #5: concurrent unique-key insert serialization */ extern bool columnar_enable_unique_lock; /* serialize same-key inserters */ @@ -741,20 +739,6 @@ extern char *ColumnarDecompressValueStream(const char *comp, uint32 compLen, int compressionType, uint32 rawLen, MemoryContext targetContext); -/* ------------------------------------------------------------------------- - * decompressed-chunk cache (columnar_cache.c, spec 8.3, 9) - * - * An optional, backend-local cache of decompressed value streams, keyed by the - * relation's storage id and the stream's absolute logical offset (both stable - * and never reused within a storage id, except across a truncate, which fires a - * relcache invalidation that flushes the whole cache). Off by default; when on - * it only avoids repeated decompression and never changes results. - * ------------------------------------------------------------------------- */ -extern void ColumnarCacheInit(void); -extern char *ColumnarGetDecompressedStream(uint64 storageId, uint64 absOffset, - const char *comp, uint32 compLen, - int compressionType, uint32 rawLen, - MemoryContext targetContext); /* ------------------------------------------------------------------------- * concurrent unique-key insert serialization (columnar_unique.c, issue #5) diff --git a/src/columnar_cache.c b/src/columnar_cache.c deleted file mode 100644 index 63f7f81..0000000 --- a/src/columnar_cache.c +++ /dev/null @@ -1,240 +0,0 @@ -/*------------------------------------------------------------------------- - * - * columnar_cache.c - * Optional decompressed-chunk cache for pgColumnar (spec 8.3, 9). - * - * When columnar.enable_column_cache is on, the reader keeps decompressed value - * streams in a backend-local cache so that repeated reads of the same chunk - * group reuse the decompressed bytes instead of decompressing again. The cache - * is a pure optimization: with it on or off, every query returns exactly the - * same rows. It is off by default and bounded by columnar.column_cache_size - * megabytes with least-recently-used eviction. - * - * Safety of the key. A cache entry is keyed by (storageId, absOffset), where - * absOffset is the value stream's absolute logical offset in the relation. A - * stripe's data is written append-only and never rewritten, so within one - * storage id an absolute offset holds exactly one immutable value stream. Two - * events can make a key stale: a truncate resets the metapage so offsets are - * reused, and vacuum swaps to a new storage id. Both fire a relcache - * invalidation, and this module flushes the entire cache on any relcache - * invalidation, so a stale entry is never read. The returned buffer is always a - * fresh copy in the caller's context, so eviction can free cache memory without - * touching a buffer a scan is still using. - * - * Independent MIT implementation built from - * design/NATIVE_FORMAT_AND_INTERFACE_SPEC.md and the public PostgreSQL API only. - * - *------------------------------------------------------------------------- - */ -#include "columnar.h" - -#include "utils/hsearch.h" -#include "utils/inval.h" -#include "utils/memutils.h" - -/* GUCs (spec 8.3); registered in columnar_tableam.c _PG_init */ -bool columnar_enable_column_cache = false; -int columnar_column_cache_size = 200; /* megabytes */ - -typedef struct ColumnarCacheKey -{ - uint64 storageId; - uint64 absOffset; /* stripe file_offset + value_stream_offset */ -} ColumnarCacheKey; - -typedef struct ColumnarCacheEntry -{ - ColumnarCacheKey key; /* must be first: dynahash hashes on it */ - uint32 rawLen; /* decompressed length */ - int compressionType; - char *buffer; /* rawLen decompressed bytes, in cacheContext */ - struct ColumnarCacheEntry *lruPrev; - struct ColumnarCacheEntry *lruNext; -} ColumnarCacheEntry; - -static HTAB *cacheHash = NULL; -static MemoryContext cacheContext = NULL; -static Size cacheBytes = 0; -static ColumnarCacheEntry *lruHead = NULL; /* most recently used */ -static ColumnarCacheEntry *lruTail = NULL; /* least recently used */ - -static void -columnar_cache_flush_all(Datum arg, Oid relid) -{ - /* - * Drop everything on any relcache invalidation. This is heavy-handed but - * always correct, and the events that matter (truncate reusing offsets, - * vacuum swapping storage) are exactly relcache invalidations. Plain reads - * do not invalidate the relcache, so a warm cache stays warm across repeated - * queries that do no DDL. - */ - if (cacheHash == NULL) - return; - - hash_destroy(cacheHash); - cacheHash = NULL; - MemoryContextReset(cacheContext); - cacheBytes = 0; - lruHead = NULL; - lruTail = NULL; -} - -void -ColumnarCacheInit(void) -{ - /* - * Create the long-lived context now; the hash table is created lazily on - * first use (and recreated after a flush). Register the invalidation - * callback once at load time. - */ - cacheContext = AllocSetContextCreate(TopMemoryContext, - "columnar decompressed cache", - ALLOCSET_DEFAULT_SIZES); - CacheRegisterRelcacheCallback(columnar_cache_flush_all, (Datum) 0); -} - -static void -columnar_cache_create_hash(void) -{ - HASHCTL info; - - MemSet(&info, 0, sizeof(info)); - info.keysize = sizeof(ColumnarCacheKey); - info.entrysize = sizeof(ColumnarCacheEntry); - info.hcxt = cacheContext; - - cacheHash = hash_create("columnar decompressed chunk cache", 128, &info, - HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); -} - -/* unlink an entry from the LRU list */ -static void -columnar_lru_unlink(ColumnarCacheEntry *e) -{ - if (e->lruPrev != NULL) - e->lruPrev->lruNext = e->lruNext; - else - lruHead = e->lruNext; - - if (e->lruNext != NULL) - e->lruNext->lruPrev = e->lruPrev; - else - lruTail = e->lruPrev; - - e->lruPrev = NULL; - e->lruNext = NULL; -} - -/* push an entry to the most-recently-used end */ -static void -columnar_lru_push_front(ColumnarCacheEntry *e) -{ - e->lruPrev = NULL; - e->lruNext = lruHead; - if (lruHead != NULL) - lruHead->lruPrev = e; - lruHead = e; - if (lruTail == NULL) - lruTail = e; -} - -static void -columnar_lru_touch(ColumnarCacheEntry *e) -{ - if (lruHead == e) - return; - columnar_lru_unlink(e); - columnar_lru_push_front(e); -} - -/* evict least-recently-used entries until within budget, never evicting keep */ -static void -columnar_cache_evict(Size budget, ColumnarCacheEntry *keep) -{ - while (cacheBytes > budget && lruTail != NULL && lruTail != keep) - { - ColumnarCacheEntry *victim = lruTail; - bool found; - - columnar_lru_unlink(victim); - cacheBytes -= victim->rawLen; - pfree(victim->buffer); - hash_search(cacheHash, &victim->key, HASH_REMOVE, &found); - } -} - -/* - * ColumnarGetDecompressedStream - * Return rawLen decompressed bytes of a value stream in targetContext. With - * the cache off, this simply decompresses. With the cache on, a hit copies - * the cached decompressed bytes (skipping decompression) and a miss - * decompresses and stores a copy for next time. The result is always a - * fresh buffer owned by the caller, so cache eviction is always safe. - */ -char * -ColumnarGetDecompressedStream(uint64 storageId, uint64 absOffset, - const char *comp, uint32 compLen, - int compressionType, uint32 rawLen, - MemoryContext targetContext) -{ - ColumnarCacheKey key; - ColumnarCacheEntry *entry; - bool found; - char *result; - Size budget; - - /* an all-null chunk has an empty value stream (spec 4) */ - if (rawLen == 0) - return NULL; - - if (!columnar_enable_column_cache) - return ColumnarDecompressValueStream(comp, compLen, compressionType, - rawLen, targetContext); - - if (cacheHash == NULL) - columnar_cache_create_hash(); - - key.storageId = storageId; - key.absOffset = absOffset; - - entry = (ColumnarCacheEntry *) hash_search(cacheHash, &key, HASH_FIND, &found); - if (found && entry->rawLen == rawLen && - entry->compressionType == compressionType) - { - result = MemoryContextAlloc(targetContext, rawLen); - memcpy(result, entry->buffer, rawLen); - columnar_lru_touch(entry); - return result; - } - - /* miss (or a defensive key collision): decompress into the caller's buffer */ - result = ColumnarDecompressValueStream(comp, compLen, compressionType, - rawLen, targetContext); - - /* do not cache a single stream larger than the whole budget */ - budget = (Size) columnar_column_cache_size * 1024L * 1024L; - if ((Size) rawLen > budget) - return result; - - /* store a copy in the cache for future reads */ - entry = (ColumnarCacheEntry *) hash_search(cacheHash, &key, HASH_ENTER, &found); - if (found) - { - /* replacing a stale entry for the same key: free its old buffer */ - columnar_lru_unlink(entry); - cacheBytes -= entry->rawLen; - pfree(entry->buffer); - } - entry->rawLen = rawLen; - entry->compressionType = compressionType; - entry->buffer = MemoryContextAlloc(cacheContext, rawLen); - memcpy(entry->buffer, result, rawLen); - entry->lruPrev = NULL; - entry->lruNext = NULL; - columnar_lru_push_front(entry); - cacheBytes += rawLen; - - columnar_cache_evict(budget, entry); - - return result; -} diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index a364f98..034ee5e 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -2253,15 +2253,6 @@ _PG_init(void) 0, NULL, NULL, NULL); - DefineCustomBoolVariable("pgcolumnar.enable_column_cache", - "Cache decompressed chunk groups to reuse across reads.", - NULL, - &columnar_enable_column_cache, - false, - PGC_USERSET, - 0, - NULL, NULL, NULL); - DefineCustomBoolVariable("pgcolumnar.reclaim_coalesce", "Split oversized freed ranges on reuse and coalesce " "adjacent freed ranges, so compaction reclaims space " @@ -2314,16 +2305,6 @@ _PG_init(void) 0, NULL, NULL, NULL); - DefineCustomIntVariable("pgcolumnar.column_cache_size", - "Size of the decompressed-chunk cache, in megabytes.", - NULL, - &columnar_column_cache_size, - 200, - 1, INT_MAX, - PGC_USERSET, - GUC_UNIT_MB, - NULL, NULL, NULL); - DefineCustomBoolVariable("pgcolumnar.enable_unique_insert_lock", "Serialize concurrent inserts of the same unique key.", "Takes a transaction-scoped advisory lock per unique " @@ -2388,7 +2369,6 @@ _PG_init(void) ColumnarVectorInit(); /* set up the optional decompressed-chunk cache (spec 8.3) */ - ColumnarCacheInit(); /* register the unique-index cache invalidation callback (issue #5) */ ColumnarUniqueInit(); diff --git a/test/phase6.sh b/test/phase6.sh index d3970af..915d23c 100644 --- a/test/phase6.sh +++ b/test/phase6.sh @@ -110,20 +110,6 @@ eq_on_off() { echo "PASS $name: $on" } -# Run a query with the decompressed-chunk cache on and off; assert equality. -cache_on_off() { - local name="$1" query="$2" - local on off - on="$(run_pg "$PSQL -c \"SET pgcolumnar.enable_column_cache=on; $query\"")" - off="$(run_pg "$PSQL -c \"SET pgcolumnar.enable_column_cache=off; $query\"")" - if [ -z "$on" ] || [ "$on" != "$off" ]; then - echo "FAIL $name: cache-on [$on] != cache-off [$off]" - fail=1 - return - fi - echo "PASS $name: $on" -} - q "CREATE EXTENSION pgcolumnar;" >/dev/null # --------------------------------------------------------------------------- @@ -207,16 +193,13 @@ eq_on_off "min empty is null" "SELECT COALESCE(min(a)::text,'NULL') FROM nt # the answer: cache on and cache off produce identical results, for aggregates # and for a filtered row scan. # --------------------------------------------------------------------------- -echo "-- decompressed-chunk cache on vs off is identical" -cache_on_off "cache agg suite" \ - "SELECT count(*), sum(id), avg(id), min(id), max(id) FROM t WHERE id > 500;" -cache_on_off "cache filtered scan" \ - "SELECT md5(string_agg(id||'|'||label, ',' ORDER BY id)) FROM t WHERE id BETWEEN 20000 AND 21000;" -cache_on_off "cache with nulls" \ - "SELECT count(a), sum(a), min(txt) FROM nt WHERE a > 50;" -# a small cache budget still returns correct results (exercises LRU eviction) -small_on="$(q "SET pgcolumnar.enable_column_cache=on; SET pgcolumnar.column_cache_size=1; SELECT sum(id) FROM t;")" -check "tiny cache still correct" "$small_on" "1250025000" +# The decompressed-chunk cache assertions that stood here are gone with the +# feature (#303). They compared results with the cache on against the cache off +# and asserted the two were equal, and one of them claimed in a comment to +# exercise LRU eviction. All three passed against a cache whose only entry point +# had no callers, because a correctness-only comparison of two identical paths +# cannot fail. If a cache returns here, its tests have to assert a hit that is +# observable and an eviction that evicts. # --------------------------------------------------------------------------- # Fallback: aggregates and column types the vectorized path does not handle must