diff --git a/CHANGELOG.md b/CHANGELOG.md index 24999969..ae42c092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -305,6 +305,45 @@ true until the next version shipped. it cannot in a suite it has never seen -- otherwise adding PG20 would redden every check at once, which is a gate somebody turns off. It tightens the moment one run on that major is merged, and it says out loud when it is not enforcing. +- The docs name 1024 as the floor for `stripe_row_limit` (#1017). + + A vector is a fixed 1024 values, so a row group smaller than one never fills it and + the chunk-shared FSST symbol table is not built. Measured on 200,000 rows of a text + column, `compression = none`, against 12,800,000 raw bytes, two identical passes: + + stripe_row_limit 1000 0 FSST tables 13,625,000 106.4% of raw + stripe_row_limit 1200 166 of 167 6,998,031 54.7% of raw + stripe_row_limit 2000 100 of 100 6,990,641 54.6% of raw + + At 1000 the column costs more than storing the bytes uncompressed. THE ACCEPTED + MINIMUM IS 1000, enforced in `set_options`, so the most aggressive legal setting is + the one that pays this -- and `docs/administration.md` tells a reader to LOWER this + setting for point-lookup-heavy tables, which is the path in. The warning now sits in + that block rather than in a reference table. + + Documentation only. The minimum is unchanged: whether to raise it, make FSST work + below a vector, or warn at `set_options` is still open on #1017. + + The chunk-group limit does not affect this, and that is now measured rather than + assumed: `chunk_group_row_limit` at its floor of 100 stores 7,086,080 bytes, the same + byte count as 1024 and 10000. Measured by @OffgridwithJD. + + THE GUARD WAS BORN GREEN TWICE BEFORE IT WORKED, and the measurement is why it does + now. A blank-line block reader passed on main, because `configuration.md`'s GUC table + has no blank lines and `stripe_row_limit`'s row shares a block with + `chunk_group_row_limit`'s "fixed 1024-value vectors". A three-line proximity window + passed for the same reason. One line naming both is 0 on all three pages on main, and + it is also a claim about the prose: the floor has to be stated in a sentence. + + The two harnesses disagreed while that was being found -- the awk arm used paragraph + mode and passed on main for two pages, the python twin split on blank lines and did + not -- which is the argument for keeping both halves, paid back the day it was written. + + AND ONE LINE SAYS NOTHING ABOUT WHERE. Moving the line out of the advice block to the + end of administration.md, 402 lines away, left the arm passing while its name claimed + the floor was stated beside the advice. Reported by @OffgridwithJD. The arm now + asserts the SECTION: the floor and the lowering advice must sit under one `## ` + heading. A heading is a declared boundary, which is what the paragraph reader lacked. - A check record names the PostgreSQL major it was observed under (#1010). diff --git a/docs/administration.md b/docs/administration.md index 0d195ac8..f2e73a2c 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -55,6 +55,12 @@ each fetch expensive. Lower this setting for a table that takes many point lookups. `pgcolumnar.chunk_group_row_limit` does not change this cost. Use `stripe_row_limit` for this, not `chunk_group_row_limit`. +Do not lower `pgcolumnar.stripe_row_limit` below **1024**. A vector is a fixed 1024 +values, so a smaller row group never fills one and FSST is not applied to text +columns. Measured at the +accepted minimum of 1000, a text column stored at 106.4% of its raw bytes against +54.7% at 1200 (#1017). Lower it to 1024 or above, not to the floor. + Measured on 500,000 rows of 1 KiB incompressible data, which is the shape where the effect is largest: diff --git a/docs/best-practices.md b/docs/best-practices.md index 54a5f4eb..f5e332a8 100644 --- a/docs/best-practices.md +++ b/docs/best-practices.md @@ -27,7 +27,10 @@ own. Small transactions produce small, poorly compressed row groups, and many of them to scan later. Prefer `COPY` or a multi-row `INSERT ... SELECT` over row-at-a-time inserts. Load in batches that fill a row group (`pgcolumnar.stripe_row_limit`, default 150000 rows), so each row group compresses -well. +well. Keep `pgcolumnar.stripe_row_limit` at 1024 or above if you lower it. A vector +is a fixed 1024 values. A row group below one vector gets no FSST on its text +columns. Those columns cost 106.4% of their raw bytes at the accepted minimum of +1000, against 54.7% at 1200 (#1017). **Use `parallel_copy` for a large file.** `pgcolumnar.parallel_copy` splits a server-side file across workers and scales the load into one table. It runs a diff --git a/docs/configuration.md b/docs/configuration.md index 4afa88a9..13a37874 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -19,7 +19,7 @@ pgColumnar has two kinds of settings: | Setting | Type | Default | Description | | --- | --- | --- | --- | -| `pgcolumnar.stripe_row_limit` | integer | `150000` | Maximum rows per row group. The row group is the unit of write and the granularity at which whole segments are appended. Range 1000 to INT_MAX. | +| `pgcolumnar.stripe_row_limit` | integer | `150000` | Maximum rows per row group. The row group is the unit of write and the granularity at which whole segments are appended. Range 1000 to INT_MAX, but see the note below: a value under 1024 costs text compression. | | `pgcolumnar.chunk_group_row_limit` | integer | `10000` | Maximum rows per chunk group. The chunk group is the band a scan skips as a unit when a filter cannot match its minimum and maximum. Within a chunk group each column is encoded in fixed 1024-value vectors. Range 100 to INT_MAX. | | `pgcolumnar.encoding_sample_rows` | integer | `2048` | The number of rows that the writer samples to select the value encoding of a vector. The writer estimates each candidate on a sample of windows. The windows contain consecutive values and have an equal distance between them. Thus the sample shows the global shape and also the local runs. The writer then applies only the two best candidates to the full vector. A value of `0` applies each candidate to each vector. This is the behaviour of earlier versions. The writer changes a value below 128 to `0`, because a smaller sample cannot put the candidates in order. This setting changes the write speed. It can also change the compression ratio. It does not change correctness. | @@ -155,7 +155,7 @@ SELECT pgcolumnar.set_options( | --- | --- | --- | | `table_name` | regclass | The columnar table to change. Anything that is not an ordinary table using the `pgcolumnar` access method is rejected, including a partitioned table. | | `chunk_group_row_limit` | integer | Per-table override of `pgcolumnar.chunk_group_row_limit`. | -| `stripe_row_limit` | integer | Per-table override of `pgcolumnar.stripe_row_limit`. | +| `stripe_row_limit` | integer | Per-table override of `pgcolumnar.stripe_row_limit`. See the note below: a value under 1024 costs text compression. | | `compression` | name | One of `none`, `pglz`, `lz4`, `zstd`. | | `compression_level` | integer | Level for the `zstd` codec, 1 to 22. | | `encode_effort` | name | `full` (default) or `fast`. How much work the writer spends choosing an encoding. See below. | @@ -163,6 +163,22 @@ SELECT pgcolumnar.set_options( | `ttl_column` | name | The `timestamp` or `timestamptz` column a retention is measured on. Set it with `ttl_interval`; neither works alone. Nothing is deleted until you call `pgcolumnar.expire(t)` by name. | | `ttl_interval` | interval | How long a row is kept, measured from `ttl_column`. `pgcolumnar.expire(t)` then drops row groups whose rows are all older than this. A group with one live row is kept whole. | +**A `stripe_row_limit` below 1024 disables FSST on text columns.** A vector is a +fixed 1024 values. A row group smaller than that never fills one, so the +chunk-shared FSST symbol table is not built. Measured on 200,000 rows of a text +column, `compression = none`, against 12,800,000 raw bytes: + +| `stripe_row_limit` | FSST tables | stored | of raw | +| --- | --- | --- | --- | +| 1000 | 0 | 13,625,000 | **106.4%** | +| 1200 | 166 of 167 | 6,998,031 | 54.7% | +| 2000 | 100 of 100 | 6,990,641 | 54.6% | + +At 1000 the column costs more than storing the bytes uncompressed. The accepted +minimum is 1000 and the vector is 1024, so **the most aggressive legal setting is +the one that pays this cost**. Use 1024 or more unless you have measured that you +want the opposite (#1017). + The function does not change an argument that keeps its default value of `NULL`. The function refuses a value that is outside the permitted range of a limit or a level. diff --git a/test/docs_style.sh b/test/docs_style.sh index f4fd677e..b664a5ac 100755 --- a/test/docs_style.sh +++ b/test/docs_style.sh @@ -148,6 +148,60 @@ PY check "best-practices names clustering on the join key" \ "$(_practices_jk)" "yes" +# ---- the stripe floor is below a vector, and the pages must say so (#1017) ---- +# +# A vector is a fixed 1024 values (COLUMNAR_NATIVE_VECTOR_LENGTH). A row group +# smaller than one never fills it and FSST is not applied to text columns. +# Measured, 200,000 rows, compression=none, against 12,800,000 raw bytes: +# +# stripe_row_limit 1000 0 FSST tables 13,625,000 106.4% of raw +# stripe_row_limit 1200 166 tables 6,998,031 54.7% of raw +# +# The ACCEPTED MINIMUM IS 1000, so the most aggressive legal setting is the one +# that pays this, and administration.md tells a reader to LOWER the setting for +# point lookups. The warning has to sit in the block that gives that advice, not +# in a reference table three pages away -- so these arms are scoped to the block +# and not to the page. An earlier version grepped whole pages and passed on main, +# which already says 1024 and FSST elsewhere. +# ONE LINE CARRYING BOTH, AND FOR administration.md THE RIGHT SECTION TOO. +# +# One line, because a blank-line block and a three-line window are both green on +# main: configuration.md's GUC table has no blank lines, so stripe_row_limit's row +# shares a block with chunk_group_row_limit's "fixed 1024-value vectors", and those +# rows are adjacent. One line naming both is 0 on all three pages on main, and it +# makes the prose state the floor in a sentence, which is what a warning needs. +# +# THE SECTION, because one line alone says nothing about WHERE. @OffgridwithJD moved +# the line out of the advice block to the end of administration.md, 402 lines away, +# and the page-wide arm still passed while its name claimed the floor was stated +# "beside the advice to lower the setting". Reproduced before changing anything. +# +# A `## ` heading is the boundary, not a blank line. That is what the paragraph +# reader got wrong: blank lines are absent inside a markdown table and arbitrary in +# prose, while a heading is declared. +_floor_line() { # _floor_line FILE -- yes if one line names the setting and the floor + if grep -qE 'stripe_row_limit.*1024|1024.*stripe_row_limit' "$1"; then + echo yes + else + echo no + fi +} +_floor_same_section() { # _floor_same_section FILE ADVICE -- yes if both are under one `## ` + awk -v advice="$2" ' + /^## / { h = substr($0, 4) } + { + if (index(tolower($0), tolower(advice))) a[h] = 1 + if (/stripe_row_limit/ && /1024/) f[h] = 1 + } + END { for (k in a) if (k in f) { print "yes"; exit } print "no" }' "$1" +} +check "configuration.md states the 1024 floor where it documents the setting" \ + "$(_floor_line "$SRCDIR/docs/configuration.md")" "yes" +check "administration.md states it in the section that says to lower the setting" \ + "$(_floor_same_section "$SRCDIR/docs/administration.md" "Lower this setting")" "yes" +check "best-practices.md carries it with the load-sizing advice" \ + "$(_floor_line "$SRCDIR/docs/best-practices.md")" "yes" + # ---- a document that quotes the version must quote the current one ---------- # # Nothing reads the VERSION file mechanically: no Makefile rule, no CI step. Two diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index fe04c582..d6891b7f 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -79,6 +79,7 @@ behaviour, the source of that number is named. - [31. test_native_ownership.py: every maintenance function is owner-only](#31-test_native_ownershippy-every-maintenance-function-is-owner-only) - [32. test_stats_privilege.py: stats is readable only by a caller who may read the table](#32-test_stats_privilegepy-stats-is-readable-only-by-a-caller-who-may-read-the-table) - [33. test_docs_table_structure.py: a table must stay a table](#33-test_docs_table_structurepy-a-table-must-stay-a-table) +- [34. test_docs_stripe_floor.py: the stripe floor is below a vector](#34-test_docs_stripe_floorpy-the-stripe-floor-is-below-a-vector) ## 1. How to read a test in here @@ -3203,3 +3204,73 @@ report that cannot say where is one somebody has to re-derive. **Measured before landing**, which is what a static guard in this tree owes. 0 across the gate's own scope, and 5 elsewhere in the tree. All five are real: 3 in this file and 2 in a design document, none of which the gate covers. + +## 34. test_docs_stripe_floor.py: the stripe floor is below a vector + +A vector is a fixed 1024 values (`COLUMNAR_NATIVE_VECTOR_LENGTH`). A row group +smaller than one never fills it, so the chunk-shared FSST symbol table is not built +and a text column is stored plain. + +Measured on 200,000 rows, one text column, `compression = none`, against 12,800,000 +raw bytes, two identical passes: + +| `stripe_row_limit` | FSST tables | stored | of raw | +| --- | --- | --- | --- | +| 1000 | 0 | 13,625,000 | **106.4%** | +| 1200 | 166 of 167 | 6,998,031 | 54.7% | +| 2000 | 100 of 100 | 6,990,641 | 54.6% | + +**The accepted minimum is 1000**, enforced in `set_options`, so the most aggressive +legal setting is the one that pays this — and at it the column costs more than +storing the bytes uncompressed. `docs/administration.md` tells a reader to *lower* +this setting for point-lookup-heavy tables, which is the path in, so the warning has +to sit in the block that gives the advice rather than in a reference table. + +### Why one line, and why the section as well + +Three signals were tried. Two were born green on `main`: + +| signal | on `main` | +| --- | --- | +| blank-line block | **passes** — `configuration.md`'s GUC table has no blank lines, so `stripe_row_limit`'s row shares a block with `chunk_group_row_limit`'s "fixed 1024-value vectors" | +| three-line window | **passes** — those rows are adjacent | +| one line naming both | **0 on all three pages** | + +One line is also a claim about the prose: the floor has to be stated in a sentence, +not inferred from two neighbouring tokens. That is why `best-practices.md` names the +setting and the number together. + +**And one line alone says nothing about WHERE.** @OffgridwithJD moved the line out of +the advice block to the end of `administration.md` — **402 lines away** — and the arm +still passed while its name claimed the floor was stated "beside the advice to lower +the setting". Reproduced here before anything changed. + +So the `administration.md` arm asserts the **section**: the floor and the lowering +advice must sit under one `## ` heading, both under `## Row-group sizing` today. A +heading is a declared boundary, which is exactly what the paragraph reader lacked — +blank lines are absent inside a markdown table and arbitrary in prose. + +| test | what it pins | +| --- | --- | +| `test_configuration_states_the_floor_where_it_documents_the_setting` | the floor is on the setting's own line | +| `test_administration_states_it_in_the_section_that_says_to_lower_it` | it is in the **same section** as the advice that leads there | +| `test_best_practices_carries_the_floor_with_the_load_sizing_advice` | the load-sizing guidance states it too | + +### Removal proof, three ways + +| mutation | result | +| --- | --- | +| `main`'s three pages | all three arms red | +| the floor line moved 402 lines from the advice | the administration arm red, the other two green | +| the branch as it stands | all three green | + +The second row is the one the page-wide version could not produce. + +**The proof itself broke once and said so.** `git stash` on the three pages stopped +reverting them the moment the change was committed rather than staged, so the +"restore main's pages" step was restoring the branch's own pages and every arm passed. +Checking the files out from `origin/main` explicitly is what makes the row mean +anything. + +The shell twin is three arms in `docs_style.sh`: `grep` for the two one-line pages and +an awk heading walker for `administration.md`. The two halves share no code. diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 72d7d613..6672bcde 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -36,12 +36,19 @@ # test_mutation_ledger.py. That is the mechanism doing its job rather than a nuisance: had # it not moved, the job would have failed with "collected 274 test(s) but expected 272" and # named the drift instead of running a different suite than the one declared. -# 277 -> 280: three arms in test_docs_cover_the_corpus.py about duplicated entries in -# VACUITY_MODES.md. Re-derived by collection rather than by adding three, per the recipe -# above: `280 tests collected`. -guard_tests 287 -# 282 -> 284: two arms about TESTS.md's own numbering, added with the fix for the -# out-of-order contents list #1023's merge left behind. Re-derived by collection. +# 277 -> 290, across four changes that landed arms over the published pages and over +# TESTS.md itself. WORTH THE PARAGRAPH, because the obvious resolution is wrong twice over. +# +# Three branches each added three arms to 277 and each derived 280, correctly, against a +# tree holding only its own three. No number was a typo and no side was stale. Keeping +# either side of such a merge ships a value no tree collects, and so does adding the deltas +# up, because the deltas were measured against different trees. +# +# RE-DERIVE BY COLLECTION on the merged tree. That is the only resolution this file has, and +# `--pgc-expect-tests` compares against exactly what collection reports: +# +# 290 tests collected +guard_tests 290 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and @@ -57,11 +64,8 @@ guard_tests 287 # which is the argument for gating it rather than a detail about it. # 166 -> 177 when test_native_ownership.py landed: nine parametrized refusal arms, # the owner control, and the check-ordering arm. +# 177 -> 205 across test_stats_privilege.py and the differential suite's growth, on top of +# test_native_ownership.py's 166 -> 177. DERIVED by collection on the merged tree, never by +# addition: several branches bumped this from 166 at once, and arithmetic on any one of +# them lands on a number no tree collects. cluster_tests 205 -# 177 -> 180 when test_stats_privilege.py landed, on top of -# test_native_ownership.py's 166 -> 177. DERIVED by collection on the merged -# tree, not 169 + 11: both branches bumped this from 166 and the union left two -# lines, which is exactly the staleness #1018's enforcement exists to catch. -# The rate is the point. The ungated half grew by 67 tests in the time #1016 took to review, -# which was the argument for gating it. It is gated now, and 166 -> 182 is the first move made -# with the gate actually watching: this number and the tests land in one commit. diff --git a/test/pytest/test_docs_stripe_floor.py b/test/pytest/test_docs_stripe_floor.py new file mode 100644 index 00000000..9e34c218 --- /dev/null +++ b/test/pytest/test_docs_stripe_floor.py @@ -0,0 +1,93 @@ +"""The docs must name 1024 as the floor for `stripe_row_limit` (#1017). + +A vector is a fixed 1024 values (`COLUMNAR_NATIVE_VECTOR_LENGTH`), so a row group +smaller than one vector never fills one and the chunk-shared FSST symbol table is +not built. Measured on 200,000 rows of a text column, `compression = none`: + + stripe_row_limit 1000 0 FSST tables 13,625,000 106.4% of raw + stripe_row_limit 1200 166 tables 6,998,031 54.7% of raw + +The accepted minimum is 1000, so the most aggressive legal setting is the one that +pays this. `docs/administration.md` tells a reader to LOWER this setting for +point-lookup-heavy tables, which is the path into it, so the warning has to live +beside that advice and not only in a reference table. + +Public seam: the three published pages. Read independently of docs_style.sh -- +this parses the pages itself rather than sharing a helper with the shell arm. +""" + +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CONFIG = ROOT / "docs" / "configuration.md" +ADMIN = ROOT / "docs" / "administration.md" +PRACTICES = ROOT / "docs" / "best-practices.md" + + +def _floor_line_sections(path): + """The `##` headings under which a line names both the setting and the floor. + + ONE LINE, because a blank-line block and a three-line window are both green on + `main`: `configuration.md`'s GUC table has no blank lines, so + `stripe_row_limit`'s row shares a block with `chunk_group_row_limit`'s "fixed + 1024-value vectors", and those rows are adjacent. One line naming both is 0 on + all three pages there, and it constrains the prose to state the floor in a + sentence, which is what a warning needs. + + AND THE SECTION, because one line alone says nothing about WHERE. Reported by + @OffgridwithJD, who moved the line out of the advice block to the end of + `administration.md` -- 402 lines away -- and the arm still passed while claiming + the floor was stated "beside the advice to lower the setting". Reproduced here + before changing anything. + + A `##` heading is the boundary, not a blank line. That is what the paragraph + reader got wrong: blank lines are absent inside a markdown table and arbitrary + in prose, while a heading is declared. + """ + out, heading = set(), None + for line in path.read_text(encoding="utf-8").splitlines(): + if line.startswith("## "): + heading = line[3:].strip() + elif "stripe_row_limit" in line and "1024" in line: + out.add(heading) + return out + + +def _sections_containing(path, needle): + out, heading = set(), None + for line in path.read_text(encoding="utf-8").splitlines(): + if line.startswith("## "): + heading = line[3:].strip() + elif needle.lower() in line.lower(): + out.add(heading) + return out + + +def test_configuration_states_the_floor_where_it_documents_the_setting(expect): + expect.num(int(CONFIG.is_file()), 1, "premise: configuration.md is in the tree") + expect.num(int(len(_floor_line_sections(CONFIG)) > 0), 1, + "configuration.md states the 1024 floor on the setting's own line") + + +def test_administration_states_it_in_the_section_that_says_to_lower_it(expect): + """`administration.md` tells a reader to LOWER this setting for point lookups. + + That is the path into the cliff, so the floor has to be in THAT section. The + arm asserts the section and not merely the page, because the page-wide version + passed with the two 402 lines apart. + """ + expect.num(int(ADMIN.is_file()), 1, "premise: administration.md is in the tree") + advice = _sections_containing(ADMIN, "lower this setting") + expect.num(int(len(advice) > 0), 1, + "premise: administration.md still tells a reader to lower the setting") + floor = _floor_line_sections(ADMIN) + expect.num(int(len(advice & floor) > 0), 1, + "and the 1024 floor is stated in that same section") + low = ADMIN.read_text(encoding="utf-8").lower() + expect.num(int("fsst" in low), 1, "and names what lowering past it costs") + + +def test_best_practices_carries_the_floor_with_the_load_sizing_advice(expect): + expect.num(int(PRACTICES.is_file()), 1, "premise: best-practices.md is in the tree") + expect.num(int(len(_floor_line_sections(PRACTICES)) > 0), 1, + "the load-sizing advice states the floor on the same line") diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index 63e8b27f..20926dc8 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -105,6 +105,9 @@ # #752 docs. Reads docs/how-to.md and docs/best-practices.md. No cluster, # no driver: the public seam is the published page. "test_docs_join_clustering.py", + # #1017 docs. Reads configuration.md, administration.md and best-practices.md. + # No cluster, no driver: the public seam is the published page. + "test_docs_stripe_floor.py", "test_docs_table_structure.py", ]