scription
+ worker detects non-transient errors (e.g. duplication error)
+ that require user intervention to resolve.
What do you think?
Regards,
Greg Nancarrow
Fujitsu Australia
ould imply the same I think.
But I don't mind if you want to explicitly state both cases to make it clear.
Regards,
Greg Nancarrow
Fujitsu Australia
On Thu, Dec 2, 2021 at 1:48 PM Greg Nancarrow wrote:
>
> If you updated my original description to say "(instead of just the
> individual partitions)", it would imply the same I think.
> But I don't mind if you want to explicitly state both cases to make it clear.
>
er_arg(newOwnerId))
as:
+ if (!superuser_arg(newOwnerId) && IsSchemaPublication(form->oid))
Regards,
Greg Nancarrow
Fujitsu Australia
ii) In the above case (where values and nulls are palloc'd),
shouldn't the values and nulls be pfree()d at the end of the function?
0005
src/backend/utils/cache/relcache.c
(1) RelationGetInvalRowFilterCol
Shouldn't "rfnode" be pfree()d after use?
Regards,
Greg Nancarrow
Fujitsu Australia
On Tue, Nov 16, 2021 at 4:38 PM Greg Nancarrow wrote:
>
> I've attached an updated patch with these changes.
>
I've attached a re-based version (no functional changes from the
previous) to fix cfbot failures.
Regards,
Greg Nancarrow
Fujitsu Australia
v23-0001-Add-a-new-lo
ion subscription \"%s\" will be disabled due
to error: %s",
+MySubscription->name, edata->message));
Perhaps a similar error message could be logged prior to EmitErrorReport()?
e.g.
"logical replication subscription \"%s\" will be disabled due to an error"
Regards,
Greg Nancarrow
Fujitsu Australia
urred for more than X minutes) or, similarly, perhaps if some
threshold has been reached (e.g. same error has occurred more than X
times), but I think that this was previously suggested by Peter Smith
and the idea wasn't looked upon all that favorably?
Regards,
Greg Nancarrow
Fujitsu Australia
hange "Create
Publication" to "CREATE PUBLICATION".
Regards,
Greg Nancarrow
Fujitsu Australia
t scenario, but I needed
to reduce the amount of inserted data (so reduced 200 to 2)
due to disk space.
I then consistently get an error like the following:
postgres=# alter database test set tablespace pg_default;
ERROR: could not create file
"pg_tblspc/16385/PG_15_202111301/16386/36395": File exists
(this only happens when the patch is used)
Regards,
Greg Nancarrow
Fujitsu Australia
ot;. I found an article that
> seems to further support this since it both sounds like a vowel (oh) and is
> also a letter (oh).
>
> https://www.grammar.com/a-vs-an-when-to-use
>
What about the "-" before the "o"?
Wouldn't it be read as "dash o" or "minus o"? This would mean "a" is
correct, not "an", IMHO.
Regards,
Greg Nancarrow
Fujitsu Australia
ip the transaction in question
+# by ALTER SUBSCRIPTION ... SKIP. Then, check if logical replication
can continue
+# working by inserting $nonconflict_data on the publisher.
(ii)
BEFORE:
+# will conflict with the data replicated from publisher later.
AFTER:
+# will conflict with the data replicated later from the publisher.
Regards,
Greg Nancarrow
Fujitsu Australia
ssue, right?
If a different xid to skip is specified while the worker is currently
skipping a transaction, should that even be allowed?
Regards,
Greg Nancarrow
Fujitsu Australia
al "*/" (so the asterisks align).
Also, should say "... treated the same".
/*
+ * If the publication is FOR ALL TABLES then it is treated same as if this
+ * table has no filters (even if for some other publication it does).
+ */
Regards,
Greg Nancarrow
Fujitsu Australia
d a way to allow users to specify only XID that has failed.
>
Yes, I agree that would be better.
If you didn't do that, I think you'd need to queue the XIDs to be
skipped (rather than locking).
Regards,
Greg Nancarrow
Fujitsu Australia
>
I was thinking such subscription requests could be rejected by the
server, based on the subscriber version and whether the publications
use filtering etc.
Regards,
Greg Nancarrow
Fujitsu Australia
ase usage on other similar entries?
(e.g. "Two phase commit")
src/include/catalog/pg_subscription.h
(3)
+ bool subdisableonerr; /* True if apply errors should disable the
+ * subscription upon error */
The comment should just say "True if occurren
arer, and the stats more meaningful?
[1]
https://www.postgresql.org/message-id/flat/cahut+pt39pbqs0sxt9rmm89ayizoq0kw46yzskkzwk8z5ho...@mail.gmail.com
Regards,
Greg Nancarrow
Fujitsu Australia
me that
the old (current) row was previously UPDATEd (and published, or not
published, according to the filter applicable to UPDATE), but this is not
necessarily the case.
Or am I missing something?
[1]
https://postgr.es/m/CAJcOf-dz0srExG0NPPgXh5X8eL2uxk7C=czogtbf8cnqoru...@mail.gmail.com
Regards,
Greg Nancarrow
Fujitsu Australia
On Fri, Dec 17, 2021 at 7:20 PM Ajin Cherian wrote:
>
> On Fri, Dec 17, 2021 at 5:46 PM Greg Nancarrow wrote:
>
> > So using the v47 patch-set, I still find that the UPDATE above results in
> > publication of an INSERT of (2,1), rather than an UPDATE of (1,1) to (2,1).
>
re sense to:
1) Disallow different WHERE clauses on the same table, for different pubactions.
2) If only INSERTs are being published, allow any column in the WHERE
clause, otherwise (as for UPDATE and DELETE) restrict the referenced
columns to be part of the replica identity or primary key.
Regards,
Greg Nancarrow
Fujitsu Australia
ssary use of a palloc'd tiny struct to return the
PublicationActions (which also currently isn't explicitly pfree'd).
Regards,
Greg Nancarrow
Fujitsu Australia
sary.
I've attached a patch which addresses that and replaces a couple of
memcpy()s with struct assignment, as suggested.
Regards,
Greg Nancarrow
Fujitsu Australia
get_rel_pubactions_improvement.patch
Description: Binary data
using the rd_pubactions pointer as
a boolean flag to indicate whether the publication membership info has
been fetched (so the bool flags are valid).
I guess you'd need another bool flag if you wanted to make that struct
in-line in the relcache entry.
Regards,
Greg Nancarrow
Fujitsu Australia
header should mention something like:
"The supplied repWorker statistics are cleared upon return, to assist
re-use by the caller."
Regards,
Greg Nancarrow
Fujitsu Australia
EFORE:
+ * It is only safe to execute UPDATE/DELETE when all columns referenced in
+ * the row filters from publications which the relation is in are valid -
AFTER:
+ * It is only safe to execute UPDATE/DELETE when all columns, referenced in
+ * the row filters from publications which the relation is in, are valid -
Regards,
Greg Nancarrow
Fujitsu Australia
d;
+ int trigtype;
should be:
+ int trigtype;
Oid tgfoid = rel->trigdesc->triggers[i].tgfoid;
(need to avoid intermingled declarations and code)
See: https://www.postgresql.org/docs/13/source-conventions.html
Regards,
Greg Nancarrow
Fujitsu Australia
l be used. This doesn't seem to be right. I think that if
PARALLEL_INSERT_CAN_IGN_TUP_COST is set, path->rows should be set to
0, and just let existing "run_cost" be evaluated as normal (which will
be 0 as path->rows is 0).
2) Is PARALLEL_INSERT_TUP_COST_IGNORED actually needed? Couldn't only
PARALLEL_INSERT_CAN_IGN_TUP_COST be used for the purpose of ignoring
parallel tuple cost?
Regards,
Greg Nancarrow
Fujitsu Australia
ly, the parallel mode is
-strictly read-only, but now we have the infrastructure to allow parallel
-inserts and parallel copy.
+mutual exclusion method for such cases. Currently, only parallel insert is
+allowed, but we have the infrastructure to allow parallel copy.
Let me know if these changes seem OK to you.
Regards,
Greg Nancarrow
Fujitsu Australia
ate->raw_buf);
This comment doesn't seem to be entirely true.
At least for text/csv file COPY FROM, cstate->raw_buf is subsequently
referenced in the SetRawBufForLoad() function, which is called by
CopyReadLineText():
cur_data_blk_ptr = (cstate->raw_buf) ?
&pcshared_info->data_blocks[cur_block_pos] : NULL;
So I think cstate->raw_buf should be set to NULL after being pfree()d, and the
comment fixed/adjusted.
(ii) This patch adds some macros (involving parallel copy checks) AFTER the
comment:
/* End parallel copy Macros */
Regards,
Greg Nancarrow
Fujitsu Australia
what other code could possibly be checking raw_buf
and using it in some way, when in fact what it points to has been
pfreed.
Are you able to add the following line of code, or will it (somehow)
break logic that you are relying on?
pfree(cstate->raw_buf);
cstate->raw_buf = NULL; <=== I suggest that this line is added
Regards,
Greg Nancarrow
Fujitsu Australia
er.
>
>But this now means there are some missing test cases for
>
>target_server_type = "any"
GN RESPONSE: Have added "any" tests for target_server_type.
>COMMENT - negative tests?
>
>IIUC when "standby" (aka "secondary") is specif
er_type == SERVER_TYPE_PREFER_STANDBY ||
> + conn->requested_server_type == SERVER_TYPE_READ_ONLY ||
> + conn->requested_server_type == SERVER_TYPE_STANDBY)))
> {
> - /* Not writable; fail this connection. */
> + if (conn->which_primary_or_rw_host == -2)
> + {
> + /*
> + * This scenario is possible only for the
> + * prefer-standby type for the next pass of the
> + * list of connections as it couldn't find any
> + * servers that are read-only.
> + */
> + goto consume_checked_target_connection;
> + }
>
> Is this goto consume_checked_target_connection deliberate?
> Previously (in the v17 patch) there was a another label, and so this
> same code did goto consume_checked_write_connection;
>
> The v17 code seems more correct than the current v18 code, which is
> now jumping to a label not even in the same case block!
>
GN RESPONSE:
Not deliberate, seems to have been messed up (possibly by copying
another block, to get a comment), but has now been corrected.
Regards,
Greg Nancarrow
Fujitsu Australia
v19-0001-Enhance-libpq-target_session_attrs-and-add-target_se.patch
Description: Binary data
test cases (reverting one to its original state
before the commit mentioned above).
Let me know what you think.
Regards,
Greg Nancarrow
Fujitsu Australia
v1-0001-Fix-GUC-parse_int-to-allow-fractional-input-only-whe.patch
Description: Binary data
QL, you can assign a fractional
> value to an integer column and the system will let you do it; so
> why not in SET?
>
> In any case, we already shipped that behavior in v12, so I don't think
> we can take it away now. People don't appreciate formerly valid
> settings suddenly not working any more.
>
I guess we'll have to live with the current behaviour then.
Regards,
Greg Nancarrow
Fujitsu Australia
ources made little or no difference to
Parallel Copy performance when the target table had no indexes.
Increasing Postgres resources improved Parallel Copy performance when
the target table had indexes.
Regards,
Greg Nancarrow
Fujitsu Australia
(1) Postgres default settings, 5GB CSV (510 rows),
Hi Vignesh,
>Can you share with me the script you used to generate the data & the ddl of
>the table, so that it will help me check that >scenario you faced the >problem.
Unfortunately I can't directly share it (considered company IP),
though having said that it's only doing something that is rel
settings makes little
difference to the performance.
Regards,
Greg Nancarrow
Fujitsu Australia
status flags to workers in ParallelWorkerMain(). I've
> attached a patch for HEAD.
>
+1 The fix looks reasonable to me too.
Is it possible for the patch to add test cases for the two identified
problem scenarios? (PROC_IN_VACUUM, PROC_IN_SAFE_IC)
Regards,
Greg Nancarrow
Fujitsu Australia
On Mon, Oct 11, 2021 at 5:39 PM vignesh C wrote:
>
> These comments are fixed in the v38 patch attached.
>
Thanks for the updates.
I noticed that these patches don't apply on the latest source (last
seemed to apply cleanly on HEAD as at about October 6).
Regards,
Greg Nan
r: Andres Freund
Date: Fri Aug 6 10:05:57 2021 -0700
pgstat: Schedule per-backend pgstat shutdown via before_shmem_exit().
Can you see if the problem can be reproduced prior to this commit?
Regards,
Greg Nancarrow
Fujitsu Australia
(5) skip_xid value validation
The validation of the specified skip_xid XID value isn't great.
For example, the following value are accepted:
ALTER SUBSCRIPTION sub SET (skip_xid='123abcz');
ALTER SUBSCRIPTION sub SET (skip_xid='99$@*');
Regards,
Greg Nancarrow
Fujitsu Australia
ge) == 0)
+ {
+ wentry->count++;
+ wentry->timestamp = msg->m_timestamp;
+ return;
+ }
Maybe the cheapest solution is to just set dbid in
pgstat_reset_subworker_error()?
src/backend/replication/logical/worker.c
(5) Fix typo
synchroniztion -> synchronization
+ * type for table synchroniztion.
Regards,
Greg Nancarrow
Fujitsu Australia
ES IN SCHEMA sch;
(6) SUB: ALTER SUBSCRIPTION sub REFRESH PUBLICATION;
(7) SUB: SELECT * FROM sch.sale;
sale_date | country_code | product_sku | units
+--+-+---
2019-01-01 | AU | cpu | 5
2019-01-02 | AU | disk| 8
2019-01-01 | AU | cpu | 5
2019-01-02 | AU | disk| 8
Regards,
Greg Nancarrow
Fujitsu Australia
er".
Suggested change:
BEFORE:
+ The parameters that can be reset are: slot_name,
+ synchronous_commit, binary,
+ streaming, and following parameter:
AFTER:
+ The parameters that can be reset are:
synchronous_commit,
+ binary, streaming, and
the following
+ parameter:
Regards,
Greg Nancarrow
Fujitsu Australia
not sure if it's a problem as such, really just a query from me as
to whether it should be allowed to also (redundantly) add partitions
to the publication, in addition to the partitioned table, since the
current documentation says: "When a partitioned table is added to a
publication, all of its existing and future partitions are implicitly
considered to be part of the publication".
I guess it should be allowed, as I find I can do it in the current
implementation just with TABLE.
Regards,
Greg Nancarrow
Fujitsu Australia
rows) had a huge performance overhead.
As an example, scantuple was originally newly allocated each row
filtered, and to filter 1,000,000 rows in a test case it was taking 40
seconds. Caching the allocation in RelationSyncEntry reduced it down
to about 5 seconds.
Regards,
Greg Nancarrow
Fujitsu Australia
27;, 'AU', 'cpu', 5),
('2019-01-02', 'AU', 'disk', 8);
(4) SUB: SELECT * FROM sch.sale;
(5) PUB: ALTER PUBLICATION pub ADD TABLE sch.sale;
(6) SUB: ALTER SUBSCRIPTION sub REFRESH PUBLICATION;
(7) SUB: SELECT * FROM sch.sale;
Regards,
Greg Nancarrow
Fujitsu Australia
; partition issue in a cleaner way.
>
A minor thing, in your "top-up patch", the test code added to
publication.sql, you need to remove the last "DROP TABLE
sch2.tbl1_part1;". It causes an error because the table doesn't exist
and it seems to have been erroneously copied from the previous test
case.
Regards,
Greg Nancarrow
Fujitsu Australia
ng transaction transaction, if enabled */
should be:
/* Stop skipping transaction changes, if enabled */
Regards,
Greg Nancarrow
Fujitsu Australia
using the identity and schema of the partitioned table" due
to publish_via_partition_root=true. Note that the corresponding table
in the subscriber may well be a non-partitioned table (or the
partitions arranged differently) so the data does need to be
replicated again.
Regards,
Greg Nancarrow
Fujitsu Australia
is
added to the publication and the subscriber does ALTER SUBSCRIPTION
... REFRESH PUBLICATION.
If I modify my example to include both the partitioned table and
(explicitly) its child partitions in the publication, and insert some
data on the publisher side prior to the subscription, then I am s
data is
replicated again.
This scenario didn't use initial table data, so initial table sync
didn't come into play (although as I previously posted, I can see a
double-publish issue on initial sync if data is put in the table prior
to subscription and partitions have been explicitly added to the
publication).
Regards,
Greg Nancarrow
Fujitsu Australia
ality isn't
correct (i.e. no data should be replicated on the REFRESH in this
case)?
I think it's debatable because here copy_data=true and sch.sale was
not a previously-subscribed table (so pre-existing data in that table
should be copied, in accordance with the current documentation).
Regards,
Greg Nancarrow
Fujitsu Australia
ions relate to ALL TABLES IN SCHEMA.
For example, if a partitioned table belongs to a different schema to
that of a child partition that belongs to a schema specified for ALL
TABLES IN SCHEMA, is it implicitly included in the publication or not
included?
Regards,
Greg Nancarrow
Fujitsu Australia
bscribed and it subscribes to the partitions. This explains why
data gets "re-copied" when this happens, because then it is
subscribing to a "new" table and copy_data=true by default.
Regards,
Greg Nancarrow
Fujitsu Australia
f partitioned tables
included in the publication if ANY schemas are included as part of the
publication (e.g. which could be a schema not including any
partitioned tables or partitions).
Regards,
Greg Nancarrow
Fujitsu Australia
tion. But I did some testing and found
that this is the current behavior when only the partitions are
individually included in a publication using TABLE, so it seems to be
OK.
Regards,
Greg Nancarrow
Fujitsu Australia
On Fri, Oct 22, 2021 at 12:41 PM Greg Nancarrow wrote:
>
> I was also previously concerned about what the behavior should be when
> only including just the partitions of a partitioned table in a
> publication using ALL TABLES IN SCHEMA and having
> publish_via_partition_root=tr
ng => 'logical');
$node_publisher->start;
# create subscriber node
-my $node_subscriber = PostgresNode->new('subscriber');
+my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber');
$node_subscriber->init(allows_streaming => 'logical');
$node_subscriber->start;
Regards,
Greg Nancarrow
Fujitsu Australia
n has occurred that we
know the XID of the failed transaction that we want to skip. So that
syntax looks a little confusing to me. Unless you had something else
in mind on how it would work?
Regards,
Greg Nancarrow
Fujitsu Australia
ff'.
>
I agree that it doesn't seem to handle the RESET of synchronous_commit.
I think that for consistency, the default value "off" for
synchronous_commit should be set (in the SubOpts) near where the
default values of the boolean supported options are currently set -
near the top of parse_subscription_options().
Regards,
Greg Nancarrow
Fujitsu Australia
filter cols are also
+* valid replica identity cols.
*/
- if (pub->pubactions.pubdelete)
+ if (pub->pubactions.pubdelete || pub->pubactions.pubupdate)
{
char replica_identity = rel->rd_rel->relreplident;
Regards,
Greg Nancarrow
Fujitsu Australia
uld be capitalized in the following line:
+JOIN pg_catalog.pg_namespace N on N.oid = S.pnnspid
(3) Some basic tests should be added for this in the publication tests.
Regards,
Greg Nancarrow
Fujitsu Australia
skipping this code if ultimately nothing is going
to be logged (and I found that even for a tiny database, a VACUUM may
execute this code hundreds of times).
I have attached a simple patch for this.
Regards,
Greg Nancarrow
Fujitsu Australia
v1-0001-Skip-vacuum-log-report-processing-in-lazy_scan_heap
tion
structure and members in SQL, without having to piece the information
together from the other pg_publication_* tables.
Personally I don't think it is necessary to additionally display all
tables in the schema (that information can be retrieved by pg_tables
or information_schema.tables, if need
r the "objtype" column, the type "schema" does not
seem specific enough; maybe that should be instead named
"all-tables-in-schema" or similar.
Regards,
Greg Nancarrow
Fujitsu Australia
e that this refers to "tablesync", so perhaps
add " (tablesync)" to the end of this sentence, or similar.
Regards,
Greg Nancarrow
Fujitsu Australia
nction.
(11) update_apply_change_size()
Shouldn't this function be declared static?
(12) stream_write_change()
+ streamed_entry->xact_size = streamed_entry->xact_size + total_len;
/* update */
could be simply written as:
+ streamed_entry->xact_size += total_len; /* update */
Regards,
Greg Nancarrow
Fujitsu Australia
On Thu, Nov 4, 2021 at 12:49 AM James Coleman wrote:
>
> Greg,
>
> Do you believe this is now ready for committer?
>
The patch LGTM.
I have set the status to "Ready for Committer".
Regards,
Greg Nancarrow
Fujitsu Australia
How about:
- If "publish_via_partition_root" is true for a publication, then data
is replicated to the table with the same name as the root (i.e.
partitioned) table in the publisher.
- If "publish_via_partition_root" is false (the default) for a
publication, then data is replicat
On Thu, Nov 4, 2021 at 7:10 PM Amit Kapila wrote:
>
> On Thu, Nov 4, 2021 at 12:23 PM Greg Nancarrow
wrote:
> >
> > On Thu, Nov 4, 2021 at 3:13 PM Amit Kapila
wrote:
> > >
> > > On further thinking about this, I think we should define the behavior
> >
#x27;m overly paranoid, but adding a backdoor of sorts for a situation
> which
> really shouldn't happen doesn't seem like a good idea.
>
>
+1
The arguments given are pretty convincing IMHO, and I agree that the
additions made in the v20 patch are not a good idea, and are not needed.
Regards,
Greg Nancarrow
Fujitsu Australia
t;stats_reset" is currently documented as the last column of the
"pg_stat_subscription_workers" view - but it's actually no longer
included in the view.
(5) src/tools/pgindent/typedefs.list
The following current entries are bogus:
PgStat_MsgSubWorkerErrorPurge
PgStat_MsgSubWorkerPurge
The following entry is missing:
PgStat_MsgSubscriptionPurge
Regards,
Greg Nancarrow
Fujitsu Australia
atch was posted a day
ago, it looks like your patch needs to be rebased against that (as it
currently applies on top of the v19 version only).
Regards,
Greg Nancarrow
Fujitsu Australia
the underlying latest v20 patch.
Regards,
Greg Nancarrow
Fujitsu Australia
tSwitchTo(ecxt);
+ }
I'm guessing it was intended to do the "MemoryContextSwitch(ecxt);"
before re-throwing (?), but it's not really clear, as in the 1st and
3rd cases, the DisableSubscriptionOnError() calls anyway immediately
switch the memory context to cctx.
Regards,
Greg Nancarrow
Fujitsu Australia
On Wed, Nov 10, 2021 at 3:22 PM Greg Nancarrow wrote:
>
> I had a look at this patch and have a couple of initial review
> comments for some issues I spotted:
>
Incidentally, I found that the v3 patch only applies after the skip xid v20
patch [1] has been applied.
On Fri, Nov 5, 2021 at 3:03 PM Greg Nancarrow wrote:
>
> +1
> The arguments given are pretty convincing IMHO, and I agree that the
> additions made in the v20 patch are not a good idea, and are not needed.
>
If there are no objections, I plan to reinstate the previous v1
and remove its restoration in pg_dump output, since it's not
needed (as in v20 patch)
- Some tidying of the updates to the event_trigger tests and
capitalization of the test SQL
Regards,
Greg Nancarrow
Fujitsu Australia
v21-0001-Add-a-new-login-event-and-login-event-trigger-support.patch
Description: Binary data
e the
potential for inconsistent data resulting in this case, unless say,
the subscriber "child tables" are first truncated on the refresh, if
they are in fact partitions of the root, and then the table sync
publishes the existing data via the root)
Regards,
Greg Nancarrow
Fujitsu Australia
; That being said, that's still not a very readable name, maybe someone else has
> an even better suggestion?
>
Yes you're right, in this case I was wrongly treating "evt" as an
abbreviation for "event".
I agree "dathasloginevt" would be better.
Regards,
Greg Nancarrow
Fujitsu Australia
DisableSubscriptionOnError() instead of the memory-context, and the
existing MemoryContextSwitch() and CopyErrorData() calls could be
removed from it.
AFAICS, applying (3) and (4) above would make the code a lot cleaner.
Regards,
Greg Nancarrow
Fujitsu Australia
line (did you run
pg_indent?)
src/include/pgstat.h
(3) Remove PgStat_StatSubWorkerEntry.dbid?
The "dbid" member of the new PgStat_StatSubWorkerEntry struct doesn't
seem to be used, so I think it should be removed.
(I could remove it and everything builds OK and tests pass).
Regards,
Greg Nancarrow
Fujitsu Australia
entry->rowfilter_valid = true;
@@ -1881,7 +1879,7 @@ rel_sync_cache_relation_cb(Datum arg, Oid relid)
}
if (entry->exprstate != NULL)
{
-free(entry->exprstate);
+pfree(entry->exprstate);
entry->exprstate = NULL;
}
}
Regards,
Greg Nancarrow
Fujitsu Australia
t being said, that's still not a very readable name, maybe someone else has
> an even better suggestion?
>
Changed to "dathasloginevt", as suggested.
I've attached an updated patch with these changes.
I also noticed one of the Windows-based cfbots was failing with an
"SS
ossible memory leak issue that may need further
investigation.
Regards,
Greg Nancarrow
the 0006 patch (when I remove that
patch, the problem doesn't occur).
Still needs investigation.
Regards,
Greg Nancarrow
Fujitsu Australia
TextDatumGetCString(rfdatum));
rfnodes = lappend(rfnodes, rfnode);
+ MemoryContextSwitchTo(oldctx);
(these changes are needed in addition to the fixes I posted on this
thread for the crash problem that was previously reported)
Regards,
Greg Nancarrow
Fujitsu Australia
rr" instead of
"disable_on_error".
Also "disable_on_err" appears in a couple of the test case comments.
Regards,
Greg Nancarrow
Fujitsu Australia
nk a test needs to be added similar to the customers+countries
example that Tomas gave (where there is a single subscription to
multiple publications of the same table, each of which has a
row-filter).
Regards,
Greg Nancarrow
Fujitsu Australia
uses "null" but the "command" column
description uses "NULL".
I think "NULL" should be used for consistency.
Regards,
Greg Nancarrow
Fujitsu Australia
Relation relation;
+ Node *whereClause;
} PublicationRelInfo;
It appears that this new member is not actually required, as the relid
can be simply obtained from the existing "relation" member - using the
RelationGetRelid() macro.
Regards,
Greg Nancarrow
Fujitsu Australia
ence messaging due to UDP
similarly mean that "first_error_time" and "last_error_time" may not
be currently handled correctly?
Regards,
Greg Nancarrow
Fujitsu Australia
>
Oops, I missed that too. So at worst, vacuum will clean it up in the
out-of-order SUBSCRIPTIONPURGE,SUBWORKERERROR case.
But I still think the current code may not correctly handle
first_error_time/last_error_time timestamps if out-of-order
SUBWORKERERROR messages occur, right?
Regard
h the most latest info.
>
Couldn't the code block in pgstat_recv_subworker_error() that
increments error_count just compare the new msg timestamp against the
existing first_error_time and last_error_time and, based on the
result, update those if required?
Regards,
Greg Nancarrow
Fujitsu Australia
not freed:
src/backend/replication/pgoutput/pgoutput.c
+ if (rfnodes)
+ {
+ list_free(rfnodes);
+ rfnodes = NIL;
+ }
Regards,
Greg Nancarrow
Fujitsu Australia
re also reported here: [1]
I've attached a patch to fix these warnings.
(Note that currently PG_USED_FOR_ASSERTS_ONLY is defined as the unused
attribute, which is only supported by GCC)
[1]:
https://www.postgresql.org/message-id/cah2-wznwwu+9on9nzcnztk7ua238mctgpxyr1ty7u_msn5z...@mail.gmail
olved
without intervention, logical replication will get stuck in a loop
retrying and the last error will occur again and again, hence the
count of how many times that has happened.
Maybe there's not much benefit in counting different errors prior to
the last error?
Regards,
Greg Nancarrow
Fujitsu Australia
USED_FOR_ASSERTS_ONLY in the same style.
>
Isn't "[[maybe_unused]]" only supported for MS C++ (not C)?
I'm using Visual Studio 17, and I get nothing but a syntax error if
trying to use it in C code, whereas it works if I rename the same
source file to have a ".cpp" extension (but even then I need to use
the "/std:c++17" compiler flag)
Regards,
Greg Nancarrow
Fujitsu Australia
101 - 200 of 431 matches
Mail list logo
