Anser bloomfilter MVP - #57
Draft
leborchuk wants to merge 48 commits into
Draft
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces the initial “Anser” adaptive information-sharing subsystem scaffolding in the backend (shared-memory channel map + GUCs + coordinator bgworkers) along with a dedicated SQL-callable test extension and regression coverage.
Changes:
- Adds core Anser shared-memory structures/APIs and wires shmem sizing/init into postmaster startup paths.
- Registers two new coordinator-only background workers and adds new Anser-related GUCs.
- Adds
src/test/modules/ansertest extension + regression test, and hooks it into both Makefile and Meson test-module builds.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/modules/meson.build | Adds the anser test module to Meson test module build. |
| src/test/modules/Makefile | Adds the anser test module to Makefile-based test module build. |
| src/test/modules/anser/test_anser.control | Declares the test_anser extension metadata. |
| src/test/modules/anser/test_anser--1.0.sql | Defines SQL wrappers for Anser test helper C functions. |
| src/test/modules/anser/sql/test_anser.sql | Adds regression SQL to exercise register/subscribe/publish/consume/cancel. |
| src/test/modules/anser/meson.build | Builds and installs test_anser for Meson; registers regression. |
| src/test/modules/anser/Makefile | Builds and installs test_anser for Makefile; registers regression. |
| src/test/modules/anser/expected/test_anser.out | Expected regression output for test_anser. |
| src/test/modules/anser/anser_test.c | Implements SQL-callable test helpers that wrap Anser APIs. |
| src/include/utils/unsync_guc_name.h | Registers new gp_anser_* GUC names in the “no sync” list. |
| src/include/postmaster/postmaster.h | Increases MaxPMAuxProc base to account for new aux workers. |
| src/include/cdb/anser.h | Adds public Anser API and shared-memory structs/GUC externs. |
| src/backend/utils/misc/guc_gp.c | Adds gp_anser_* GUC definitions and includes Anser header. |
| src/backend/storage/lmgr/lwlocknames.txt | Adds two LWLock names for Anser. |
| src/backend/storage/ipc/ipci.c | Hooks Anser shmem sizing/init into shared-memory creation paths. |
| src/backend/storage/ipc/ipc.c | Changes proc_exit() gprof directory chdir() handling. |
| src/backend/postmaster/postmaster.c | Registers two new Anser background workers in PMAuxProcList. |
| src/backend/cdb/Makefile | Adds anser to cdb backend subdirectories. |
| src/backend/cdb/anser/Makefile | Adds Makefile for building Anser backend objects. |
| src/backend/cdb/anser/anserservice.c | Adds (currently skeletal) gather/send background worker mains. |
| src/backend/cdb/anser/anser.c | Implements Anser shared-memory channel map and core APIs. |
| src/backend/cdb/anser/anser-pr1-plan.md | Adds in-repo design/plan documentation for PR1. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+162
to
+175
| int32 gp_session_id = PG_GETARG_INT32(0); | ||
| int32 gp_command_count = PG_GETARG_INT32(1); | ||
| uint32 condition_id = (uint32) PG_GETARG_INT32(2); | ||
| char *condition_key = text_to_cstring(PG_GETARG_TEXT_PP(3)); | ||
|
|
||
| if (key == NULL) | ||
| return false; | ||
|
|
||
| MemSet(key, 0, sizeof(AnserChannelKey)); | ||
| key->gp_session_id = gp_session_id; | ||
| key->gp_command_count = gp_command_count; | ||
| key->condition_id = condition_id; | ||
| strlcpy(key->condition_key, condition_key, ANSER_CONDITION_KEY_SIZE); | ||
| return true; |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
src/backend/cdb/anser/anserclient.c:424
- AnserClientConsumeWait() allocates
bufeven when the caller passespayload == NULL, which will leak memory for every successful delivery in that call pattern (e.g., if a caller only cares aboutcancelled).
int len = PQgetlength(res, 0, 0);
char *val = PQgetvalue(res, 0, 0);
void *buf = NULL;
if (len > 0)
{
buf = palloc(len);
memcpy(buf, val, len);
}
if (payload != NULL)
*payload = buf;
if (payload_len != NULL)
*payload_len = (Size) len;
ok = true;
src/backend/cdb/anser/anser-pr1-plan.md:251
- The Anser PR plan doc says
gp_anser_max_info_sizedefaults to1MB, but the GUC is defined with a default of16 * 1024 * 1024in src/backend/utils/misc/guc_gp.c. This mismatch will mislead operators sizing shared memory.
| `gp_anser_enable` | `off` | `PGC_POSTMASTER` | Master switch. When off: **no shmem allocated, no workers registered**; enabling requires a restart. |
| `gp_anser_max_channels` | `128` | `PGC_POSTMASTER` | Channel hash size / arena slot count (sizes shmem) |
| `gp_anser_max_info_size` | `1MB` | `PGC_POSTMASTER` | Per-record memory cap, per the paper (sizes shmem) |
| `gp_anser_timeout_ms` | `1000` | `PGC_USERSET` | Weak-dependency wait budget for consumers |
src/backend/cdb/anser/anserplanexec.c:465
- Fail-open is expected to happen in benign cases (e.g., cancellation/timeouts or partial delivery). Logging it at LOG will spam server logs for any query that hits this path, potentially at high frequency when gp_anser_runtime_filter is enabled. This should be DEBUG-level or behind an explicit debug GUC.
if (st->filter == NULL)
elog(LOG,
"anser bloom consumer: no filter, failing open (cancelled=%d, received_parts=%u)",
ExecAnserBloomFilterConsumerWasCancelled(st->consume),
ExecAnserBloomFilterConsumerReceivedParts(st->consume));
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
That's the MVP for the Anser https://vldb.org/pvldb/vol16/p3636-wu.pdf
Here we covered only scenario with adding bloomfilters to the query execution plan.
The main idea is as a follow
The overall execution plan should looks like
The main architecture overview
Open questions
PlannedStmt * plannerwas modified and special hookAnserApplyRuntimeFilterswas added. We generate execution plan and after that add to it special steps for gathering/redistribution runtime data. Is it Ok or we should fix the planner too?How to enable && test it