Re: Disable WAL logging to speed up data loading
I read the patch and have two points: 1. I do basebackup for database then switch wal level from logical to none to logical and of cause I archive the wal segments. Next I do PITR base on the basebackup, as a result it success startup with a waring said maybe data missed. Because the 'none' level is to bulkload data, do you think it's good that we still support recover from a 'none' wal level. 2. I just mark wal_level as 'none' but fail to startup, it success after I drop the publication and it's subscription,mark max_wal_senders as 0, drop replicate slot. I think it worth to write how we can startup a 'none' wal level database in document .
Re: row filtering for logical replication
Hello
I find several problems as below when I test the patches:
1. There be some regression problem after apply 0001.patch~0005.patch
The regression problem is solved in 0006.patch
2. There be a data wrong after create subscription if the relation contains
inherits table, for example:
##
The Tables:
CREATE TABLE cities (
nametext,
population float,
altitudeint
);
CREATE TABLE capitals (
state char(2)
) INHERITS (cities);
Do on publication:
insert into cities values('aaa',123, 134);
insert into capitals values('bbb',123, 134);
create publication pub_tc for table cities where (altitude > 100 and
altitude < 200);
postgres=# select * from cities ;
name | population | altitude
--++--
aaa |123 | 134
bbb |123 | 134
(2 rows)
Do on subscription:
create subscription sub_tc connection 'host=localhost port=5432
dbname=postgres' publication pub_tc;
postgres=# select * from cities ;
name | population | altitude
--++--
aaa |123 | 134
bbb |123 | 134
bbb |123 | 134
(3 rows)
##
An unexcept row appears.
3. I am puzzled when I test the update.
Use the tables in problem 2 and test as below:
#
On publication:
postgres=# insert into cities values('t1',123, 34);
INSERT 0 1
postgres=# update cities SET altitude = 134 where altitude = 34;
UPDATE 1
postgres=# select * from cities ;
name | population | altitude
--++--
t1 |123 | 134
(1 row)
On subscription:
postgres=# select * from cities ;
name | population | altitude
--++--
(0 rows)
On publication:
insert into cities values('t1',1,'135');
update cities set altitude=300 where altitude=135;
postgres=# table cities ;
name | population | altitude
--++--
t1 |123 | 134
t1 | 1 | 300
(2 rows)
On subscription:
ostgres=# table cities ;
name | population | altitude
--++--
t1 | 1 | 135
(1 row)
#
Result1:Update a row that is not suitable the publication condition to
suitable, the subscription change nothing.
Result2: Update a row that is suitable for the publication condition to
not suitable, the subscription change nothing.
If it is a bug? Or there should be an explanation about it?
4. SQL splicing code in fetch_remote_table_info() function is too long
---
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead...@highgo.ca
The new status of this patch is: Waiting on Author
Re: Rearranging ALTER TABLE to avoid multi-operations bugs
> This review seems not very on-point, because I made no claim to have fixed
> any of those bugs. The issue at the moment is how to structure the code
I am sorry for that and I have another question now. I researched the related
code and find something as below:
Code:
~~
case AT_AddIdentity:
{
...
attnum = get_attnum(relid, cmd->name);
/*
* if attribute not found, something will error about it
* later
*/
if (attnum != InvalidAttrNumber)
generateSerialExtraStmts(&cxt, newdef,
get_atttype(relid, attnum),def->options, true,
NULL, NULL);
...
}
~~
Test case1:
create table t10 (f1 int);
alter table t10 add column f2 int not null,
alter column f2 add generated always as identity;
I find that the value of 'attnum' is 0 because now we do not have the 'f2'
column when I run the Test case1, so it can not generate a sequence
(because it can not run the generateSerialExtraStmts function).
You can see the code annotation that 'something will error about it later',
so I thank it may be an error report instead of executing successfully.
Test case2:
create table t11 (f1 int);
alter table t11 add column f2 int,
alter column f2 type int8;
Code about 'alter column type' have the same code annotation, and
if you run the Test case2, then you can get an error report. I use Test case2
to prove that it may be an error report instead of executing successfully.
--
Movead.Li
The new status of this patch is: Waiting on Author
Re: Append with naive multiplexing of FDWs
Hello Kyotaro, >"Parallel scan" at the moment means multiple workers fetch unique >blocks from *one* table in an arbitrated manner. In this sense >"parallel FDW scan" means multiple local workers fetch unique bundles >of tuples from *one* foreign table, which means it is running on a >single session. That doesn't offer an advantage. It maybe not "parallel FDW scan", it can be "parallel shards scan" the local workers will pick every foreign partition to scan. I have ever draw a picture about that you can see it in the link below. https://www.highgo.ca/2019/08/22/parallel-foreign-scan-of-postgresql/ I think the "parallel shards scan" make sence in this way. >If parallel query processing worked in worker-per-table mode, >especially on partitioned tables, maybe the current FDW would work >without much of modification. But I believe asynchronous append on >foreign tables on a single process is far resource-effective and >moderately faster than parallel append. As the test result, current patch can not gain more performance when it returns a huge number of tuples. By "parallel shards scan" method, it can work well, because the 'parallel' can take full use of CPUs while 'asynchronous' can't. Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca/ EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
Re: Asynchronous Append on postgres_fdw nodes.
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
I occur a strange issue when a exec 'make installcheck-world', it is:
##
...
== running regression test queries==
test adminpack... FAILED 60 ms
==
1 of 1 tests failed.
==
The differences that caused some tests to fail can be viewed in the
file "/work/src/postgres_app_for/contrib/adminpack/regression.diffs". A copy
of the test summary that you see
above is saved in the file
"/work/src/postgres_app_for/contrib/adminpack/regression.out".
...
##
And the content in 'contrib/adminpack/regression.out' is:
##
SELECT pg_file_write('/tmp/test_file0', 'test0', false);
ERROR: absolute path not allowed
SELECT pg_file_write(current_setting('data_directory') || '/test_file4',
'test4', false);
- pg_file_write
- 5
-(1 row)
-
+ERROR: reference to parent directory ("..") not allowed
SELECT pg_file_write(current_setting('data_directory') || '/../test_file4',
'test4', false);
ERROR: reference to parent directory ("..") not allowed
RESET ROLE;
@@ -149,7 +145,7 @@
SELECT pg_file_unlink('test_file4');
pg_file_unlink
- t
+ f
(1 row)
##
However the issue does not occur when I do a 'make check-world'.
And it doesn't occur when I test the 'make installcheck-world' without the
patch.
The new status of this patch is: Waiting on Author
Re: Asynchronous Append on postgres_fdw nodes.
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested I redo the make installcheck-world as Kyotaro Horiguchi point out and the result nothing wrong. And I think the patch is good in feature and performance here is the test result thread I made before: https://www.postgresql.org/message-id/CA%2B9bhCK7chd0qx%2Bmny%2BU9xaOs2FDNJ7RaxG4%3D9rpgT6oAKBgWA%40mail.gmail.com The new status of this patch is: Ready for Committer
Re: recovery_target_action=pause with confusing hint
Hello, When I test the patch, I find an issue: I start a stream with 'promote_trigger_file' GUC valid, and exec pg_wal_replay_pause() during recovery and as below it shows success to pause at the first time. I think it use a initialize 'SharedPromoteIsTriggered' value first time I exec the pg_wal_replay_pause(). # postgres=# select pg_wal_replay_pause(); pg_wal_replay_pause - (1 row) postgres=# select pg_wal_replay_pause(); ERROR: standby promotion is ongoing HINT: pg_wal_replay_pause() cannot be executed after promotion is triggered. postgres=# select pg_wal_replay_pause(); ERROR: recovery is not in progress HINT: Recovery control functions can only be executed during recovery. postgres=# ## The new status of this patch is: Waiting on Author
Re: POC and rebased patch for CSN based snapshots
Thanks for the remarks, >Some remarks on your patch: >1. The variable last_max_csn can be an atomic variable. Yes will consider. >2. GenerateCSN() routine: in the case than csn < csnState->last_max_csn >This is the case when someone changed the value of the system clock. I >think it is needed to write a WARNING to the log file. (May be we can do >synchronization with a time server. Yes good point, I will work out a way to report the warning, it should exist a report gap rather than report every time it generates CSN. If we really need a correct time? What's the inferiority if one node generate csn by monotonically increasing? >3. That about global snapshot xmin? In the pgpro version of the patch we >had GlobalSnapshotMapXmin() routine to maintain circular buffer of >oldestXmins for several seconds in past. This buffer allows to shift >oldestXmin in the past when backend is importing global transaction. >Otherwise old versions of tuples that were needed for this transaction >can be recycled by other processes (vacuum, HOT, etc). >How do you implement protection from local pruning? I saw >SNAP_DESYNC_COMPLAIN, but it is not used anywhere. I have researched your patch which is so great, in the patch only data out of 'global_snapshot_defer_time' can be vacuum, and it keep dead tuple even if no snapshot import at all,right? I am thanking about a way if we can start remain dead tuple just before we import a csn snapshot. Base on Clock-SI paper, we should get local CSN then send to shard nodes, because we do not known if the shard nodes' csn bigger or smaller then master node, so we should keep some dead tuple all the time to support snapshot import anytime. Then if we can do a small change to CLock-SI model, we do not use the local csn when transaction start, instead we touch every shard node for require their csn, and shard nodes start keep dead tuple, and master node choose the biggest csn to send to shard nodes. By the new way, we do not need to keep dead tuple all the time and do not need to manage a ring buf, we can give to ball to 'snapshot too old' feature. But for trade off, almost all shard node need wait. I will send more detail explain in few days. >4. The current version of the patch is not applied clearly with current >master. Maybe it's because of the release of PG13, it cause some conflict, I will rebase it. --- Regards, Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca/ EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
Re: A patch for get origin from commit_ts.
>Thanks. Movead, please note that the patch is waiting on author? >Could you send an update if you think that those changes make sense? Thanks for approval the issue, I will send a patch at Monday. Regards, Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca/ EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
Re: [PATCH] Implement INSERT SET syntax
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed It builds failed by applying to the latest code version, and I try head '73025140885c889410b9bfc4a30a3866396fc5db' which work well. The new status of this patch is: Waiting on Author
POC and rebased patch for CSN based snapshots
Hello hackers, I have read the community mail from 'postgrespro' which the link below ①, a summary for the patch, it generals a CSN by timestamp when a transaction is committed and assigns a special value as CSN for abort transaction, and record them in CSN SLRU file. Now we can judge if a xid available in a snapshot with a CSN value instead of by xmin,xmax and xip array so that if we hold CSN as a snapshot which can be export and import. CSN may be a correct direction and an important part to implement distributed of PostgreSQL because it delivers few data among cross-nodes for snapshot, so the patch is meant to do some research. We want to implement Clock-SI base on the patch.However the patch is too old, and I rebase the infrastructure part of the patch to recently commit(7dc37ccea85). The origin patch does not support csn alive among database restart because it will clean csnlog at every time the database restart, it works well until a prepared transaction occurs due to the csn of prepare transaction cleaned by a database restart. So I add wal support for csnlog then csn can alive all the time, and move the csnlog clean work to auto vacuum. It comes to another issue, now it can't switch from a xid-base snapshot to csn-base snapshot if a prepare transaction exists because it can not find csn for the prepare transaction produced during xid-base snapshot. To solve it, if the database restart with snapshot change to csn-base I record an 'xmin_for_csn' where start to check with csn snapshot. Some issues known about the current patch: 1. The CSN-snapshot support repeatable read isolation level only, we should try to support other isolation levels. 2. We can not switch fluently from xid-base->csn-base, if there be prepared transaction in database. What do you think about it, I want try to test and improve the patch step by step. ①https://www.postgresql.org/message-id/21BC916B-80A1-43BF-8650-3363CCDAE09C%40postgrespro.ru --- Regards, Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca/ EMAIL: mailto:movead(dot)li(at)highgo(dot)ca 0001-CSN-base-snapshot.patch Description: Binary data 0002-Wal-for-csn.patch Description: Binary data
Re: Asynchronous Append on postgres_fdw nodes.
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: tested, passed Spec compliant: not tested Documentation:not tested I have tested the feature and it shows great performance in queries which have small amount result compared with base scan amount.
Re: Rearranging ALTER TABLE to avoid multi-operations bugs
I applied the 'alter-table-with-recursive-process-utility-calls-wip.patch' on the master(e788e849addd56007a0e75f3b5514f294a0f3bca). And when I test the cases, I find it works well on 'alter table t1 add column f2 int not null, alter column f2 add generated always as identity' case, but it doesn't work on #14827, #15180, #15670, #15710. ~~ Here is the test result with #14827 failed ~~ postgres=# create table t10 (f1 int); CREATE TABLE postgres=# alter table t10 add column f2 int not null, postgres-# alter column f2 add generated always as identity; ALTER TABLE postgres=# postgres=# insert into t10 values(0); INSERT 0 1 postgres=# create table test_serial ( teststring varchar(5)); CREATE TABLE postgres=# alter table test_serial add column if not exists uid BIGSERIAL; ALTER TABLE postgres=# alter table test_serial add column if not exists uid BIGSERIAL; psql: NOTICE: column "uid" of relation "test_serial" already exists, skipping ALTER TABLE postgres=# postgres=# \d List of relations Schema | Name | Type |Owner +--+--+-- public | t10 | table| lichuancheng public | t10_f2_seq | sequence | lichuancheng public | test_serial | table| lichuancheng public | test_serial_uid_seq | sequence | lichuancheng public | test_serial_uid_seq1 | sequence | lichuancheng (5 rows) postgres=# ~~ So it's wrong with a 'test_serial_uid_seq1' sequence to appear. The new status of this patch is: Waiting on Author
