Hi,
On 2022-10-12 12:27:54 -0700, Andres Freund wrote:
> I intentionally put my changes into a fixup commit, in case you want to look
> at the differences.
I pushed the (combined) patch now. Thanks for your contribution!
Greetings,
Andres Freund
Hi,
On 2022-09-28 18:19:57 +0300, Melih Mutlu wrote:
> diff --git a/contrib/pg_buffercache/pg_buffercache--1.3--1.4.sql
> b/contrib/pg_buffercache/pg_buffercache--1.3--1.4.sql
> new file mode 100644
> index 00..77e250b430
> --- /dev/null
> +++ b/contrib/pg_buffercache/pg_buffercache--1.3-
Hi,
On Sep 28, 2022, 23:20 +0800, Melih Mutlu , wrote:
> Hi,
>
> Seems like the commit a448e49bcbe40fb72e1ed85af910dd216d45bad8 reverts the
> changes on pg_buffercache.
>
> > Why compute usagecount_avg twice?
> Then, I'm going back to v11 + the fix for this.
>
> Thanks,
> Melih
Looks good to me.
Hi,
Seems like the commit a448e49bcbe40fb72e1ed85af910dd216d45bad8 reverts the
changes on pg_buffercache.
Why compute usagecount_avg twice?
>
Then, I'm going back to v11 + the fix for this.
Thanks,
Melih
v14-0001-Added-pg_buffercache_summary-function.patch
Description: Binary data
Hi,
On Sep 28, 2022, 22:41 +0800, Melih Mutlu , wrote:
>
>
> Zhang Mingli , 28 Eyl 2022 Çar, 17:31 tarihinde şunu
> yazdı:
> > Why compute usagecount_avg twice?
>
> I should have removed the first one, but I think I missed it.
> Nice catch.
>
> Attached an updated version.
>
> Thanks,
> Melih
>
H
Zhang Mingli , 28 Eyl 2022 Çar, 17:31 tarihinde şunu
yazdı:
> Why compute usagecount_avg twice?
>
I should have removed the first one, but I think I missed it.
Nice catch.
Attached an updated version.
Thanks,
Melih
v13-0001-Added-pg_buffercache_summary-function.patch
Description: Binary data
Regards,
Zhang Mingli
On Sep 28, 2022, 21:50 +0800, Melih Mutlu , wrote:
> Hi all,
>
> The patch needed a rebase due to recent changes on pg_buffercache.
> You can find the updated version attached.
>
> Best,
> Melih
>
>
```
+
+ if (buffers_used != 0)
+ usagecount_avg = usagecount_avg / buff
Hi all,
The patch needed a rebase due to recent changes on pg_buffercache.
You can find the updated version attached.
Best,
Melih
v12-0001-Added-pg_buffercache_summary-function.patch
Description: Binary data
Hi Andres,
Adjusted the patch so that it will work with meson now.
Also addressed your other reviews as well.
I hope explanations in comments/docs are better now.
Best,
Melih
v11-0001-Added-pg_buffercache_summary-function.patch
Description: Binary data
Hi,
On 2022-09-22 18:22:44 +0300, Melih Mutlu wrote:
> Since header locks are removed again, I put some doc changes and comments
> back.
Due to the merge of the meson build system, this needs to adjust meson.build
as well.
> --- a/contrib/pg_buffercache/expected/pg_buffercache.out
> +++ b/contr
Hi,
Since header locks are removed again, I put some doc changes and comments
back.
Thanks,
Melih
v10-0001-Added-pg_buffercache_summary-function.patch
Description: Binary data
Hi Andres,
> All you need to do is to read BufferDesc->state into a local variable and
> then make decisions based on that
You are right, thanks.
Here is the corrected patch.
--
Best regards,
Aleksander Alekseev
v9-0001-Added-pg_buffercache_summary-function.patch
Description: Binary data
Hi,
On 2022-09-20 12:45:24 +0300, Aleksander Alekseev wrote:
> > I'm not sure how to avoid any undefined behaviour without locks though.
> > Even with locks, performance is much better. But is it good enough for
> > production?
>
> Potentially you could avoid taking locks by utilizing atomic
> op
Hi,
On Sep 20, 2022, 20:49 +0800, Melih Mutlu , wrote:
> Hi Zhang,
>
> Those are two different locks.
> The locks that are taken in the patch are for buffer headers. This locks only
> the current buffer and makes that particular buffer's info consistent within
> itself.
>
> However, the lock ment
Hi Zhang,
Those are two different locks.
The locks that are taken in the patch are for buffer headers. This locks
only the current buffer and makes that particular buffer's info consistent
within itself.
However, the lock mentioned in the doc is for buffer manager which would
prevent changes on a
Hi,
Regards,
Zhang Mingli
On Sep 20, 2022, 20:43 +0800, Aleksander Alekseev ,
wrote:
>
> Correct, the procedure doesn't take the locks of the buffer manager.
> It does take the locks of every individual buffer.
Ah, now I get it, thanks.
Hi Zhang,
> The doc says we don’t take lock during pg_buffercache_summary, but I see
> locks in the v8 patch, Isn’t it?
>
> ```
> Similar to pg_buffercache_pages function
> pg_buffercache_summary doesn't take buffer manager
> locks [...]
> ```
Correct, the procedure doesn't take the locks of t
Hi,
Correct me if I’m wrong.
The doc says we don’t take lock during pg_buffercache_summary, but I see locks
in the v8 patch, Isn’t it?
```
Similar to pg_buffercache_pages function
pg_buffercache_summary doesn't take buffer manager
locks, thus the result is not consistent across all buffers. T
Hi,
Seems like cfbot tests are passing now:
https://cirrus-ci.com/build/4727923671302144
Best,
Melih
Melih Mutlu , 20 Eyl 2022 Sal, 14:00 tarihinde şunu
yazdı:
> Aleksander Alekseev , 20 Eyl 2022 Sal, 13:57
> tarihinde şunu yazdı:
>
>> There was a missing empty line in pg_buffercache.out which
Aleksander Alekseev , 20 Eyl 2022 Sal, 13:57
tarihinde şunu yazdı:
> There was a missing empty line in pg_buffercache.out which made the
> tests fail. Here is a corrected v8 patch.
>
I was just sending a corrected patch without the missing line.
Thanks a lot for all these reviews and the correct
Hi hackers,
> Here is a corrected patch v7. To me it seems to be in pretty good
> shape, unless cfbot and/or other hackers will report any issues.
There was a missing empty line in pg_buffercache.out which made the
tests fail. Here is a corrected v8 patch.
--
Best regards,
Aleksander Alekseev
Hi Melih,
> I changed these names and updated the patch.
Thanks for the updated patch!
> Aleksander, do you still think the average usagecount is a bit useless? Or
> does it make sense to you to keep it like this?
I don't mind keeping the average.
> I'm not sure how to avoid any undefined beh
Hi,
Also I suggest changing the names of the columns in order to make them
> consistent with the rest of the system. If you consider pg_stat_activity
> and family [1] you will notice that the columns are named
> (entity)_(property), e.g. backend_xid, backend_type, client_addr, etc. So
> instead of
Hi,
On 2022-09-09 17:36:45 +0300, Aleksander Alekseev wrote:
> I suggest we focus on saving the memory first and then think about the
> performance, if necessary.
Personally I think the locks part is at least as important - it's what makes
the production impact higher.
Greetings,
Andres Freund
Hello Aleksander,
> I'm not sure about what undefined behaviour could harm this badly.
>
> You are right that in practice nothing wrong will (probably) happen on
> x86/x64 architecture with (most?) modern C compilers. This is not true in
> the general case though. It's up to the compiler to decide
Hi Melih,
> I'm not sure about what undefined behaviour could harm this badly.
You are right that in practice nothing wrong will (probably) happen on
x86/x64 architecture with (most?) modern C compilers. This is not true in
the general case though. It's up to the compiler to decide how reading th
Hi Aleksander and Nathan,
Thanks for your comments.
Aleksander Alekseev , 9 Eyl 2022 Cum, 17:36
tarihinde şunu yazdı:
> However I'm afraid you can't examine BufferDesc's without taking
> locks. This is explicitly stated in buf_internals.h:
>
> """
> Buffer header lock (BM_LOCKED flag) must be he
Hi hackers,
> > I suggest we focus on saving the memory first and then think about the
> > performance, if necessary.
>
> +1
I made a mistake in v3 cfbot complained about. It should have been:
```
if (RelFileNumberIsValid(BufTagGetRelNumber(&bufHdr->tag)))
```
Here is the corrected patch.
--
On Fri, Sep 09, 2022 at 05:36:45PM +0300, Aleksander Alekseev wrote:
> However I'm afraid you can't examine BufferDesc's without taking
> locks. This is explicitly stated in buf_internals.h:
Yeah, when I glanced at this patch earlier, I wondered about this.
> I suggest we focus on saving the memo
Hi Melih,
> I would appreciate any feedback/comment on this change.
Another benefit of pg_buffercache_summary() you didn't mention is that
it allocates much less memory than pg_buffercache_pages() does.
Here is v3 where I added this to the documentation. The patch didn't
apply to the current mas
Hi hackers,
I also added documentation changes into the patch.
You can find it attached.
I would appreciate any feedback about this pg_buffercache_summary function.
Best,
Melih
From 82e92d217dd240a9b6c1184cf29d4718343558b8 Mon Sep 17 00:00:00 2001
From: Melih Mutlu
Date: Tue, 9 Aug 2022 16:42:
31 matches
Mail list logo
