gpdb
gpdb copied to clipboard
Set sync/unsync flag for gucs
The GUC's name must be populated into sync_guc_name.h if it needs to sync value between QD and QEs.
QD will dispatch its current synced GUC values(as startup options) to create QEs. Otherwise, the
settings will not take effect on the newly created QE.
An example of GUC inconsistency between QD and QE:
CREATE OR REPLACE FUNCTION cleanupAllGangs() RETURNS BOOL
AS '@abs_builddir@/../regress/regress.so', 'cleanupAllGangs' LANGUAGE C;
CREATE OR REPLACE FUNCTION public.segment_setting(guc text) RETURNS SETOF text EXECUTE ON ALL SEGMENTS AS
$$ BEGIN RETURN NEXT pg_catalog.current_setting(guc); END $$ LANGUAGE plpgsql;
postgres=# show allow_segment_DML;
allow_segment_DML
-------------------
off
(1 row)
postgres=# set allow_segment_DML = on;
SET
postgres=# show allow_segment_DML;
allow_segment_DML
-------------------
on
(1 row)
postgres=# select public.segment_setting('allow_segment_DML');
segment_setting
-----------------
on
on
on
(3 rows)
postgres=# select cleanupAllGangs();
cleanupallgangs
-----------------
t
(1 row)
postgres=# show allow_segment_DML;
allow_segment_DML
-------------------
on
(1 row)
postgres=# select public.segment_setting('allow_segment_DML');
segment_setting
-----------------
off
off
off
(3 rows)
Here are some reminders before you submit the pull request
- [ ] Add tests for the change
- [ ] Document changes
- [ ] Communicate in the mailing list if needed
- [ ] Pass
make installcheck - [ ] Review a PR in return to support the community
Side notes while reviewing this PR (which should be tackled as separate PR)
- GUC
gp_enable_slow_writer_testmodeshould be replaced by fault injector mechanism to test - GUC
gp_instrument_shmem_sizestill relevant? - GUC
gp_safefswritesize, what exactly is the purpose of this? Do we really need it max_appendonly_tablesno more a GUC delete the same from file - we really need some mechanism to find stale entries (similar to missing entries) in these sync/unsync files- GUC
gp_heap_require_relhasoids_matchandtest_AppendOnlyHash_eviction_vs_just_marking_not_inuseare unused in code and hence should be deleted from guc_gp.c
Thanks @QingMa-gp for starting this effort on GUCs, really important GPDB7 readiness task. Once this PR is wrapped up, next we should look into flags assigned for each and every GUC specially the ones inherited from PostgreSQL and make sure it aligns in GPDB (like if GUC is not relevant shouldn't be exposed). If exposed then should be working fine in GPDB providing stated functionally.
Sorry @QingMa-gp I found an old issue: https://github.com/greenplum-db/gpdb/issues/6703
Not sure if my previous comment is correct.
Sorry @QingMa-gp I found an old issue: #6703
Not sure if my previous comment is correct.
Thanks @kainwen . I did some tests based on the master branch. allow_system_table_mods should synchronize to all QEs.
postgres=# SHOW allow_system_table_mods;
allow_system_table_mods
-------------------------
on
(1 row)
postgres=# select public.segment_setting('allow_system_table_mods');
segment_setting
-----------------
off
off
off
(3 rows)
postgres=# alter table pg_toast.pg_toast_16389 RENAME TO pg_toast_renamed;
ERROR: permission denied: "pg_toast_16389" is a system catalog (seg0 10.117.190.237:7002 pid=30458)
all these Debug_appendonly_* GUCs need to be in sync file
Done.