Re: Flushing large data immediately in pqcomm

David Rowley , 6 Nis 2024 Cmt, 04:34 tarihinde şunu yazdı: > Does anyone else want to try the attached script on the v5 patch to > see if their numbers are better? > I'm seeing the below results with your script on my machine (). I ran it several times, results were almost similar each time.

Re: Flushing large data immediately in pqcomm

Jelte Fennema-Nio , 4 Nis 2024 Per, 16:34 tarihinde şunu yazdı: > On Thu, 4 Apr 2024 at 13:08, Melih Mutlu wrote: > > I changed internal_flush() to an inline function, results look better > this way. > > It seems you also change internal_flush_buffer to be inline (but on

Re: Flushing large data immediately in pqcomm

Hi, Melih Mutlu , 28 Mar 2024 Per, 22:44 tarihinde şunu yazdı: > > On Wed, Mar 27, 2024 at 14:39 David Rowley wrote: >> >> On Fri, 22 Mar 2024 at 12:46, Melih Mutlu wrote: >> can you confirm if the test was done in debug with casserts on? If >> so, it would be m

Re: Parent/child context relation in pg_get_backend_memory_contexts()

that particular name, is actually included in the path. I couldn't find any other information that is unique to each context. Thanks, -- Melih Mutlu Microsoft

Re: Separate memory contexts for relcache and catcache

vignesh C , 27 Oca 2024 Cmt, 06:01 tarihinde şunu yazdı: > On Wed, 3 Jan 2024 at 16:56, Melih Mutlu wrote: > CFBot shows that the patch does not apply anymore as in [1]: > === Applying patches on top of PostgreSQL commit ID > 729439607ad210dbb446e31754e8627d7e3f7dda === > ===

Re: Flushing large data immediately in pqcomm

On Wed, Mar 27, 2024 at 18:54 Robert Haas wrote: > On Wed, Mar 27, 2024 at 7:39 AM David Rowley wrote: > > Robert, I understand you'd like a bit more from this patch. I'm > > wondering if you planning on blocking another committer from going > > ahead with this? Or if you have a reason why the

Re: Flushing large data immediately in pqcomm

On Wed, Mar 27, 2024 at 14:39 David Rowley wrote: > On Fri, 22 Mar 2024 at 12:46, Melih Mutlu wrote: > > I did all of the above changes and it seems like those resolved the > regression issue. > > Thanks for adjusting the patch. The numbers do look better, but on > lo

Re: Flushing large data immediately in pqcomm

Hi, PSA v3. Jelte Fennema-Nio , 21 Mar 2024 Per, 12:58 tarihinde şunu yazdı: > On Thu, 21 Mar 2024 at 01:24, Melih Mutlu wrote: > > What if I do a simple comparison like PqSendStart == PqSendPointer > instead of calling pq_is_send_pending() > > Yeah, that sounds worth tryi

Re: Flushing large data immediately in pqcomm

Heikki Linnakangas , 14 Mar 2024 Per, 15:46 tarihinde şunu yazdı: > On 14/03/2024 13:22, Melih Mutlu wrote: > > @@ -1282,14 +1283,32 @@ internal_putbytes(const char *s, size_t len) > > if (internal_flush()) > >

Re: Flushing large data immediately in pqcomm

David Rowley , 21 Mar 2024 Per, 00:54 tarihinde şunu yazdı: > On Fri, 15 Mar 2024 at 02:03, Jelte Fennema-Nio > wrote: > > > > On Thu, 14 Mar 2024 at 13:12, Robert Haas wrote: > > > > > > On Thu, Mar 14, 2024 at 7:22 AM Melih Mutlu > wrote: > &g

Re: Flushing large data immediately in pqcomm

) not buffering at all starts performing better than HEAD. Similarly the patch performs better too as it stops buffering if data does not fit into the buffer. [1] https://www.postgresql.org/message-id/CAGECzQTYUhnC1bO%3DzNiSpUgCs%3DhCYxVHvLD2doXNx3My6ZAC2w%40mail.gmail.com Thanks, -- Melih Mutlu

Re: Flushing large data immediately in pqcomm

Just to clarify, I don't think anyone here suggests that the bar should be at "if it can't lose relative to today, it's good enough". IMHO "a change that improves some cases, but regresses nowhere" does not translate to that. Thanks, -- Melih Mutlu Microsoft

Re: Flushing large data immediately in pqcomm

the data into PqSendBuffer only if the socket would block and cannot send it. Unfortunately, I don't have strong numbers to demonstrate any improvement in perf or timing yet. But I still like to know what would you think about it? Thanks, -- Melih Mutlu Microsoft

Re: Flushing large data immediately in pqcomm

, NOW() AS time FROM generate_series(1, 100) AS i; [2] rm /tmp/dummy && echo 3 | sudo tee /proc/sys/vm/drop_caches && time psql -d postgres -c "COPY test TO STDOUT;" > /tmp/dummy Thanks, -- Melih Mutlu Microsoft

Re: Improve WALRead() to suck data directly from WAL buffers when possible

pasting this into the most places we call WalRead(). Wouldn't it be better if we mention this somewhere around WALRead() only once? Best, -- Melih Mutlu Microsoft

Re: Parent/child context relation in pg_get_backend_memory_contexts()

ew should be the same, > I think. > I've done what you suggested. Also moved "total_bytes_including_children" right after "total_bytes". 14dd0f27d have introduced new macro foreach_int. > It seems to be able to make the code a bit simpler and the commit log > says this macro is primarily intended for use in new code. For example: > Makes sense. Done. Thanks, -- Melih Mutlu Microsoft v5-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch Description: Binary data

Re: Parent/child context relation in pg_get_backend_memory_contexts()

../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:282: > > trailing whitespace. >select count(*) > 0 > > ../patch/pg_backend_memory_context_refine/v3-0001-Adding-id-parent_id-into-pg_backend_memory_contex.patch:283: > &g

Re: Separate memory contexts for relcache and catcache

That would be nice. Do you have a suggestion about where that right place would be? I'd just delete these comments, they're just pointlessly restating the code. > Done. Thanks, -- Melih Mutlu Microsoft v2-0001-Separate-memory-contexts-for-relcache-and-catcach.patch Description: Binary data

Flushing large data immediately in pqcomm

time FROM generate_series(1, 1) AS i; Thanks, -- Melih Mutlu Microsoft 0001-Flush-large-data-immediately-in-pqcomm.patch Description: Binary data

Re: Parent/child context relation in pg_get_backend_memory_contexts()

( > (SELECT sum(total_bytes) FROM contexts WHERE ARRAY[a.id] <@ path) > + total_bytes > ), > total_bytes > ) AS total_bytes_including_children > FROM contexts a; > I added a "total_bytes_including_children" column as you suggested. Did th

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

nk v26-0001 will perform 84% worse than HEAD, if you try again? I just want to be sure that it was not a random thing. Interestingly, I also don't see an improvement in above results as big as in your results when inserts/tx ratio is smaller. Even though it certainly is improved in such cases. Thanks, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

]. It makes sense that those issues can cause this problem here. It just takes a bit of time for me to figure out these things, but I'm working on it. [1] https://www.postgresql.org/message-id/CALDaNm1TA068E2niJFUR9ig%2BYz3-ank%3Dj5%3Dj-2UocbzaDnQPrA%40mail.gmail.com Thanks, -- Melih Mutlu Microsoft

Separate memory contexts for relcache and catcache

pg_backend_memory_contexts WHERE name LIKE '%CacheMemoryContext%' OR parent LIKE '%CacheMemoryContext%' GROUP BY name ORDER BY total_bytes DESC; Thanks, -- Melih Mutlu Microsoft 0001-Separate-memory-contexts-for-relcache-and-catcache.patch Description: Binary data

Re: Parent/child context relation in pg_get_backend_memory_contexts()

Hi hackers, Melih Mutlu , 16 Haz 2023 Cum, 17:03 tarihinde şunu yazdı: > With this change, here's a query to find how much space used by each > context including its children: > > > WITH RECURSIVE cte AS ( > > SELECT id, total_bytes, id as root, name as r

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Peter Smith , 3 Ağu 2023 Per, 12:06 tarihinde şunu yazdı: > Just to clarify my previous post, I meant we will need new v26* patches > Right. I attached the v26 as you asked. Thanks, -- Melih Mutlu Microsoft v26-0001-Reuse-Tablesync-Workers.patch Description: Binary data v26-0002-Reus

Re: Adding a LogicalRepWorker type field

= (LogicalRepWorker *) lfirst(lc); > > - if (isParallelApplyWorker(w)) > + if (is_worker_type(w, TYPE_PARALLEL_APPLY_WORKER)) > logicalrep_worker_stop_internal(w, SIGTERM); >} Regards, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi, > PFA an updated version with some of the earlier reviews addressed. Forgot to include them in the previous email. Thanks, -- Melih Mutlu Microsoft v24-0003-Reuse-connection-when-tablesync-workers-change-t.patch Description: Binary data v24-0002-Reuse-Tablesync-Workers.patch Descript

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

o 0002 with a slight change as follows: - send_feedback(last_received, false, false); > + if (!MyLogicalRepWorker->relsync_completed) > + send_feedback(last_received, false, false); IMHO relsync_completed means simply the same with streaming_done, that's why I wanted to check that flag instead o

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

efer to assert in the very beginning but am_tablesync_worker and am_leader_apply_worker require MyLogicalRepWorker to be not NULL. And MyLogicalRepWorker is assigned in logicalrep_worker_attach. I can change this if you think there is a better way to check the worker type. Thanks, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi, Melih Mutlu , 21 Tem 2023 Cum, 12:47 tarihinde şunu yazdı: > I did not realize the order is the same with .c files. Good to know. I'll > fix it along with other comments. > Addressed the recent reviews and attached the updated patches. Thanks, -- Melih Mutlu Microsoft v22-0001

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

eep all the new function names to > > follow _ style. > > > > Fixing the naming inconsistency will be more far-reaching than just a > few functions affected by these "reuse" patches. There are plenty of > existing functions already inconsistently named in the HEAD code. So > perhaps this topic should be moved to a separate thread? > +1 for moving it to a separate thread. This is not something particularly introduced by this patch. Thanks, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

; > +1. Also, note that they should be in the same order as they are in .c > files. > I did not realize the order is the same with .c files. Good to know. I'll fix it along with other comments. Thanks, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

ne once. > ~ > Is this paragraph even needed? Since the connection is reused then it > already implies the other end (the Wlasender) is being reused, right? I actually see no harm in explaining this explicitly. Thanks, -- Melih Mutlu Microsoft v21-0001-Refactor-to-split-Apply-and-Tablesync-Workers.patch Description: Binary data v21-0002-Reuse-Tablesync-Workers.patch Description: Binary data v21-0003-Reuse-connection-when-tablesync-workers-change-t.patch Description: Binary data

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

decided to use "logical replication worker" regardless of the worker's type [1]. That's why I made this change. If that's not the case here, I'll put it back. [1] https://www.postgresql.org/message-id/flat/CAHut%2BPt1xwATviPGjjtJy5L631SGf3qjV9XUCmxLu16cHamfgg%40mail.gmail.com Thanks, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

gresql.org/message-id/CAGPVpCTvALKEXe0%3DN-%2BiMmVxVQ-%2BP8KZ_1qQ1KsSSZ-V9wJ5hw%40mail.gmail.com Thanks for the reminder. -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

he publisher. I'm not sure how that would also affect the performance if there were any writes. Thanks, -- Melih Mutlu Microsoft

Re: Changing types of block and chunk sizes in memory contexts

so > the fundamental limitation that MemoryChunk can't store block offsets > larger than 1GBs anyway, so things will go bad if we tried to have > blocks bigger than 1GB. > Right! I don't know why I cast them to Size. Thanks for the fix. Best, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

ven master beats design#2 in some cases though. Not sure if that is expected or there are some places to improve design#2 even more. What do you think? PS: I only attached the related patches and not the whole patch set. 0001 and 0002 may contain some of your earlier reviews, but I'll send a proper updated set s

Re: Changing types of block and chunk sizes in memory contexts

t; > - Afaics AllocSet->keeper is unnecessary these days, as it is always allocated > together with the context itself. Saves 8 bytes. This seemed like a safe change and removed the keeper field in AllocSet and Generation contexts. It saves an additional 8 bytes. Best, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

rs spend less time with actually copying data but more with other stuff like launching workers, opening connections etc. > * 0003 basically improved performance from first two patches Agree, 0003 is definitely a good addition which was missing earlier. Thanks, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi, Amit Kapila , 6 Tem 2023 Per, 06:56 tarihinde şunu yazdı: > > On Wed, Jul 5, 2023 at 1:48 AM Melih Mutlu wrote: > > > > Hayato Kuroda (Fujitsu) , 4 Tem 2023 Sal, > > 08:42 tarihinde şunu yazdı: > > > > > But in the later patch the tablesync worker tr

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

he publisher for creating a slot or only a snapshot does not really make enough difference. I was hoping that creating only snapshot by an existing replication slot would help the performance. I guess I was either wrong or am missing something in the implementation. The tricky bit with keeping a long t

Changing types of block and chunk sizes in memory contexts

ontexts. I tried to find most of the places that needed to be changed to uint32, but I probably missed some. I can add more places if you feel like it. I would appreciate any feedback. Thanks, -- Melih Mutlu Microsoft 0001-Change-memory-context-fields-to-uint32.patch Description: Binary data

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

y cannot be put there? I'm not really against moving those functions to tablesync.c. But what's not clear to me is worker.c. Is it the places to put common functions for all log. rep. workers? Then, what about apply worker? Should we consider a separate file for apply worker too? Thanks, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

only at the end of sync workers. How do you think? I tried to move the logicalrep_worker_wakeup call from clean_sync_worker (end of an iteration) to finish_sync_worker (end of sync worker). I made table sync much slower for some reason, then I reverted that change. Maybe I should look a bit more int

Parent/child context relation in pg_get_backend_memory_contexts()

n case of total_bytes including children. Maybe, we can also consider adding such frequently used and/or useful information as new fields in pg_get_backend_memory_contexts() too. I appreciate any comment/feedback on this. Thanks, -- Melih Mutlu Microsoft 0001-Adding-id-parent_id-into-pg_backend_memory_contexts.patch Description: Binary data

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi Peter, Peter Smith , 26 May 2023 Cum, 10:30 tarihinde şunu yazdı: > > On Thu, May 25, 2023 at 6:59 PM Melih Mutlu wrote: > Yes, I was mostly referring to the same as point 1 below about patch > 0001. I guess I just found the concept of mixing A) launching TSW (via > apply

Re: Refactor ssl tests to avoid using internal PostgreSQL::Test::Cluster methods

make it look like start()/stop(). What do you think? > command_fails( > [ 'pg_ctl', '-D', $node->data_dir, '-l', $node->logfile, 'restart' ], > 'restart fails with incorrect SSL protocol bounds'); There are two other places where ssl tests restart the node like above. We

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

5 ms | ++-+-+-++ So yes, increasing the number of workers makes it faster. But reusing workers can still improve more. [1] https://www.postgresql.org/message-id/CAAKRu_YKGyF%2BsvRQqe1th-mG9xLdzneWgh9H1z1DtypBkawkkw%40mail.gmail.com [2] https://www.postgresql.org/message-id/CAGPVpCRWEVhXa7ovrhuSQofx4to7o22oU9iKtrOgAOtz_%3DY6vg%40mail.gmail.com [3] https://www.postgresql.org/message-id/CAGPVpCRzD-ZZEc9ienhyrVpCzd9AJ7fxE--OFFJBnBg3E0438w%40mail.gmail.com Best, -- Melih Mutlu Microsoft

Re: Subscription statistics are not dropped at DROP SUBSCRIPTION in some cases

action fails for some reason, as you explained. IMO, the patch will be OK after commit message is added. Thanks, -- Melih Mutlu Microsoft

Re: pg_stat_io for the startup process

the timer needs to be armed? Or need to add an additional global flag to rearm the timer? Any thoughts? [1] https://api.cirrus-ci.com/v1/artifact/task/5282291971260416/testrun/build/testrun/recovery/010_logical_decoding_timelines/log/010_logical_decoding_timelines_replica.log Best, -- Melih Mutlu Microsoft v2-0001-Add-timeout-to-flush-stats-during-startup-s-main-replay-loop.patch Description: Binary data

Re: pg_stat_io for the startup process

ead? 2- I'm also not sure if this timeout should be registered at the beginning of StartupProcessMain, or does it even make any difference. I tried to do this just before the main redo loop in PerformWalRecovery, but that made the CI red. Best, -- Melih Mutlu Microsoft 0001-Add-timeout-to-flush-stats-during-startup-s-main-replay-loop.patch Description: Binary data

Re: Allow logical replication to copy tables in binary format

Hi, Amit Kapila , 23 Mar 2023 Per, 08:48 tarihinde şunu yazdı: > Pushed the 0001. It may be better to start a separate thread for 0002. > Great! Thanks. Best, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

make1. Amit Kapila , 21 Mar 2023 Sal, 12:27 tarihinde şunu yazdı: > > Explaining the issue explicitly with a comment seems better to me than > the trick of changing order of table creation just for some test cases. > > But I'm also ok with removing the use of disable_on_error if

Re: Allow logical replication to copy tables in binary format

er without using > disable_on_error option in these tests. > While this change would make the test inconsistent, I think it also would make more confusing. Explaining the issue explicitly with a comment seems better to me than the trick of changing order of table creation just for some test cases. But I'm also ok with removing the use of disable_on_error if that's what you agree on. Thanks, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

Hi, Please see the attached patch. vignesh C , 18 Mar 2023 Cmt, 12:03 tarihinde şunu yazdı: > On Fri, 17 Mar 2023 at 17:55, Melih Mutlu wrote: > 1) Currently we refer the link to the beginning of create subscription > page, this can be changed to refer to binary option contents

Re: Allow logical replication to copy tables in binary format

somehow didn't include those lines into the patch. Thanks for noticing. Fixed them now. Thanks, -- Melih Mutlu Microsoft v17-0001-Allow-logical-replication-to-copy-table-in-binary.patch Description: Binary data

Re: Allow logical replication to copy tables in binary format

supporting this for >= > v16. > Upgrading the subscriber to v16 and keeping the subscriber in v14 could break existing subscriptions. I don't know how likely such a case is. I don't have a strong preference on this. What do you think? Should we change it >=v16 or keep it as it is? Best, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

ould be better to pass the log offset when using wait_for_log, > because otherwise it will check the whole log file to find the target > message, > This might not be a big problem, but it has a risk of getting unexpected > log message > which was generated by previous commands. > You're rig

Re: Allow logical replication to copy tables in binary format

d data on the subscriber > +$result = $node_subscriber->safe_psql('postgres', 'SELECT a,b FROM > +public.test_col_order;'); > + > +is( $result, '1|2 > +3|4', 'check synced data on subscriber for different column order'); > Right, it needs to be ordered. Fixed. Thanks, -- Melih Mutl

Re: Allow logical replication to copy tables in binary format

Amit Kapila , 15 Mar 2023 Çar, 12:31 tarihinde şunu yazdı: > On Tue, Mar 14, 2023 at 4:32 PM Melih Mutlu > wrote: > > What purpose does this test serve w.r.t this patch? Before checking > the sync for different column orders, the patch has already changed > binary to false,

Re: Allow logical replication to copy tables in binary format

d until v14 it was decided to check using the later version. > Changed it as suggested here [2]. [1] https://www.postgresql.org/message-id/CAGPVpCTaXYctCUp3z%3D_BstonHiZcC5Jj7584i7B8jeZQq4RJkw%40mail.gmail.com [2] https://www.postgresql.org/message-id/CAA4eK1%2BC7ykvdBxh_t1BdbX5Da1bM1BgsE%3D-i2koPkd3pSid0A%40mail.gmail.com Thanks, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

t; for example. Kindly please fix it. > Changed this with Amit's suggestion [1]. [1] https://www.postgresql.org/message-id/CAA4eK1%2BC7ykvdBxh_t1BdbX5Da1bM1BgsE%3D-i2koPkd3pSid0A%40mail.gmail.com Thanks, -- Melih Mutlu Microsoft v14-0001-Allow-logical-replication-to-copy-table-in-binary.patch Description: Binary data

Re: Allow logical replication to copy tables in binary format

added those tests into 014_binary.pl. Also added the case with different column order as Peter suggested. Best, -- Melih Mutlu Microsoft v13-0001-Allow-logical-replication-to-copy-table-in-binary.patch Description: Binary data

Re: Allow logical replication to copy tables in binary format

Hi, Attached v12 with a unified option. Setting binary = true now allows the initial sync to happen in binary format. Thanks, -- Melih Mutlu Microsoft v12-0001-Allow-logical-replication-to-copy-table-in-binary.patch Description: Binary data

Re: Allow logical replication to copy tables in binary format

e version checks? I removed any kind of check since it’s currently a different option. Should we check publisher version before doing binary copy to ensure that the publisher node supports binary option of COPY command? Thanks, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

ore likely happen to > fail > the initial copy user should enable binary format after synchronization if > tables have original datatype. > I tried to explain when binary copy can cause failures in the doc. What exactly do you think is missing? Best, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

binary_copy option which is not tied to anything, explanations in the doc and separate tests etc. But I also agree that binary=true should make everything in binary and binary=false should do them in text format. It makes more sense. Best, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

took effect. > > Another way to test that BINARY is enabled could be to trigger one > of the failure cases. Yes, there is already a failure case for binary copy which resolves with swithcing binary_copy to false. But I also added checks for publisher logs now too. [1] https://www.postgresql.org/docs/devel/sql-copy.html Thanks, -- Melih Mutlu Microsoft v11-0001-Allow-logical-replication-to-copy-table-in-binary.patch Description: Binary data

Re: Allow logical replication to copy tables in binary format

%40mail.gmail.com Thanks, -- Melih Mutlu Microsoft v10-0001-Allow-logical-replication-to-copy-table-in-binar.patch Description: Binary data

Re: Allow logical replication to copy tables in binary format

t even if copy_data=false. Another way to deal with this issue could be expecting the user to specify format every time copy_format is needed, similar to the case for copy_data, and moving on with text (default) format if it's not specified for the current CREATE/ALTER SUBSCRIPTION execution. But I don't think this would make things easier. Best, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

PLICATION_SLOT" and "CREATE_REPLICATION_SNAPSHOT" are expected since one creates a new replication slot and the other does not. CreateDecodingContext is also used in other places as well. Not sure how this change would affect those places. I'll look into this more. Please let me kn

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi Shveta, Thanks for reviewing. Please see attached patches. shveta malik , 2 Şub 2023 Per, 14:31 tarihinde şunu yazdı: > On Wed, Feb 1, 2023 at 5:37 PM Melih Mutlu wrote: > for (int64 i = 1; i <= lastusedid; i++) > { > charori

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi, Melih Mutlu , 16 Şub 2023 Per, 14:37 tarihinde şunu yazdı: > I see that setting originname in the catalog before actually creating it > causes issues. My concern with setting originname when setting the state to > FINISHEDCOPY is that if worker waits until FINISHEDCOPY to upda

Re: Allow logical replication to copy tables in binary format

Amit Kapila , 16 Şub 2023 Per, 15:47 tarihinde şunu yazdı: > On Mon, Jan 30, 2023 at 4:19 PM Melih Mutlu > wrote: > >> 8. Note that the COPY binary format isn't portable across platforms > >> (Windows to Linux for instance) or major versions > >> https://

Re: Allow logical replication to copy tables in binary format

Hi, Hayato Kuroda (Fujitsu) , 20 Şub 2023 Pzt, 10:12 tarihinde şunu yazdı: > Dear Melih, > > Thank you for updating the patch. Before reviewing, I found that > cfbot have not accepted v8 patch [1]. > Thanks for letting me know. Attached the fixed version of the patch. Best,

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

of such failures between origin creation and FINISHEDCOPY. One fix I can think is to update the catalog right after creating a new origin. But this would also require commiting the current transaction to actually persist the originname. I guess this action of commiting the transaction in the middle of initial sync could hurt the copy process. What do you think? Also; working on an updated patch to address your other comments. Thanks again. Best, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

me know your status. > Sorry for the delay. This patch is currently one of my priorities. Hopefully I will share quicker updates from now on. [1] https://www.postgresql.org/message-id/CAGPVpCQYi9AYQSS%3DRmGgVNjz5ZEnLB8mACwd9aioVhLmbgiAMA%40mail.gmail.com Thanks, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

Hi, Please see the attached patch for following changes. Bharath Rupireddy , 30 Oca 2023 Pzt, 15:34 tarihinde şunu yazdı: > On Mon, Jan 30, 2023 at 4:19 PM Melih Mutlu > wrote: It'd be better and clearer to have a separate TAP test file IMO since > the focus of the feature here isn'

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi Shveta, shveta malik , 1 Şub 2023 Çar, 15:01 tarihinde şunu yazdı: > On Wed, Feb 1, 2023 at 5:05 PM Melih Mutlu wrote: > 2) I found a crash in the previous patch (v9), but have not tested it > on the latest yet. Crash happens when all the replication slots are > consumed and w

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

for pointing out this review. I somehow skipped that, sorry. Please see attached patches. shiy.f...@fujitsu.com , 17 Oca 2023 Sal, 10:46 tarihinde şunu yazdı: > On Wed, Jan 11, 2023 4:31 PM Melih Mutlu wrote: > 0001 patch > > 1. walsender.c > + /* Create a

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

n builder->initial_xmin_horizon but the slot is > carrying Invalid one. > I think you're right. Moved GetOldestSafeDecodingTransactionId call before xmin_horizon assignment. Thanks, -- Melih Mutlu Microsoft v6-0001-Add-replication-protocol-cmd-to-create-a-snapshot.patch Description: Binary data v10-0002-Reuse-Logical-Replication-Background-worker.patch Description: Binary data

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi, Please see attached patches for the below changes. shveta malik , 27 Oca 2023 Cum, 13:12 tarihinde şunu yazdı: > On Thu, Jan 26, 2023 at 7:53 PM Melih Mutlu > wrote: > 1. > --Can we please add a few more points to the Summary to make it more clear. > a) something telling t

Re: Allow logical replication to copy tables in binary format

Hi Bharath, Thanks for reviewing. Bharath Rupireddy , 18 Oca 2023 Çar, 10:17 tarihinde şunu yazdı: > On Thu, Jan 12, 2023 at 1:53 PM Melih Mutlu > wrote: > 1. The performance numbers posted upthread [1] look impressive for the > use-case tried, that's a 2.25X improvement or 55.

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi Shveta, Thanks for reviewing. shveta malik , 25 Oca 2023 Çar, 16:02 tarihinde şunu yazdı: > On Mon, Jan 23, 2023 at 6:30 PM Melih Mutlu > wrote: > --I see initial data copied, but new catalog columns srrelslotname > and srreloriginname are not updated: > postgres=# select sub

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi, Thanks for your review. Attached updated versions of the patches. wangw.f...@fujitsu.com , 17 Oca 2023 Sal, 14:15 tarihinde şunu yazdı: > On Wed, Jan 11, 2023 4:31 PM Melih Mutlu wrote: > v3-0001* patch > === > > 1. typedefs.list > I think w

Re: Time delayed LR (WAS Re: logical replication restrictions)

see that subscription.sql already tests this ALTER SUBSCRIPTION behaviour. Best, -- Melih Mutlu Microsoft

Re: Allow logical replication to copy tables in binary format

Hi, Thanks for your reviews. Takamichi Osumi (Fujitsu) , 12 Oca 2023 Per, 06:07 tarihinde şunu yazdı: > On Wednesday, January 11, 2023 7:45 PM Melih Mutlu > wrote: > (1-1) There is no need to log the version and the query by elog here. > (1-2) Also, I suggest we can remove the se

Re: Allow logical replication to copy tables in binary format

Hi, Thanks for your review. shiy.f...@fujitsu.com , 11 Oca 2023 Çar, 11:56 tarihinde şunu yazdı: > On Mon, Nov 14, 2022 8:08 PM Melih Mutlu wrote: > 1. > +# Binary enabled subscription should fail > +$node_subscriber_binary->wait_for_log("ERROR: insufficient da

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi hackers, Rebased the patch to resolve conflicts. Best, -- Melih Mutlu Microsoft v3-0001-Add-replication-protocol-cmd-to-create-a-snapshot.patch Description: Binary data v7-0002-Reuse-Logical-Replication-Background-worker.patch Description: Binary data

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

Hi hackers, Sending an updated version of this patch to get rid of compiler warnings. I would highly appreciate any feedback. Thanks, -- Melih Mutlu Microsoft v2-0001-Add-replication-protocol-cmd-to-create-a-snapshot.patch Description: Binary data v6-0002-Reuse-Logical-Replication

Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns

state->opts.force_null_all) >attnums = cstate->attnumlist; > else >attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->opts.force_null); Thanks, -- Melih Mutlu Microsoft

Re: Apply worker fails if a relation is missing on subscriber even if refresh publication has not been refreshed yet

issue will also be visible in the logs, just not as an error. And if users decide/remember to refresh the publication, they cannot do that anyway if the table is still missing on the subscriber. So the REFRESH PUBLICATION command will fail and then users will see an error log. Best, -- Melih Mutlu Microsoft

Apply worker fails if a relation is missing on subscriber even if refresh publication has not been refreshed yet

there until the next REFRESH PUBLICATION, missing those tables won't be a problem for existing subscriptions. [1] https://www.postgresql.org/docs/current/sql-altersubscription.html Thanks, -- Melih Mutlu Microsoft 0001-Do-not-apply-for-tables-which-not-exist-on-catalog.patch Description: Binary data

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

MB | 1 GB -- master | 27751.463 ms | 312424.999 ms -- patch | 27342.760 ms | 310021.767 ms Best, -- Melih Mutlu Microsoft

Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication

expected. I would appreciate any feedback/thought on the approach/patch/numbers etc. Thanks, -- Melih Mutlu Microsoft v2-0001-Add-replication-protocol-cmd-to-create-a-snapshot.patch Description: Binary data v5-0002-Reuse-Logical-Replication-Background-worker.patch Description: Binary data

Re: [PATCH] psql: Add tab-complete for optional view parameters

;(") ? /* ALTER VIEW */ > else if (Matches("ALTER", "VIEW", MatchAny)) > COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", > "SET SCHEMA"); Also seems like SET and RESET don't get auto-completed for "ALTER VIEW ". I think it would be nice to include those missing words. Thanks, -- Melih Mutlu Microsoft

Re: wake up logical workers after ALTER SUBSCRIPTION

hough. Just asking since currently the patch wakes up all workers including sync workers if any still exists. Best, -- Melih Mutlu Microsoft

Re: Improve tab completion for ALTER FUNCTION/PROCEDURE/ROUTINE

Hi, vignesh C , 6 Ara 2022 Sal, 22:12 tarihinde şunu yazdı: > I did not make it as a macro for alter function options as it is used > only in one place whereas the others were required in more than one > place. > Okay, makes sense. I tested the patch and it worked for me. Best, --

Re: wake up logical workers after ALTER SUBSCRIPTION

state is SYNCDONE and the apply worker has to wake up to change the state to READY. I think we already call logicalrep_worker_wakeup_ptr wherever it's needed for the above cases? What am I missing here? Best, -- Melih Mutlu Microsoft

  1   2   >