On Tuesday, April 5, 2022, Andres Freund <
andres@anarazel.de> wrote:
Hi,
On 2022-04-05 08:49:36 -0700, David G. Johnston wrote:
> On Mon, Apr 4, 2022 at 7:36 PM Andres Freund <andres@anarazel.de> wrote:
>
> >
> > I think all this is going to achieve is to making code more complicated.
> > There
> > is a *single* non-assert use of accessed_across_databases and now a single
> > assertion involving it.
> >
> > What would having PGSTAT_KIND_CLUSTER and PGSTAT_KIND_DATABASE achieve?
> >
>
> So, I decided to see what this would look like; the results are attached,
> portions of it also inlined below.
> I'll admit this does introduce a terminology problem - but IMO these words
> are much more meaningful to the reader and code than the existing
> booleans. I'm hopeful we can bikeshed something agreeable as I'm strongly
> in favor of making this change.
Sorry, I just don't agree. I'm happy to try to make it look better, but this
isn't it.
Do you think it should be your way strongly enough that you'd not want to get
it in the current way?
Not that strongly; I’m good with the code as-is. Its not pervasive enough to be hard to understand (I may ponder some code comments though) and the system it is modeling has some legacy aspects that are more the root problem and I don’t want to touch those here for sure.
Oring PGSTAT_CLUSTER = 2 with PGSTAT_OBJECT = 3 yields 3 again. To do this
kind of thing the different values need to have power-of-two values, and then
the tests need to be done with &.
Thanks.
Nicely demonstrated by the fact that with the patch applied initdb doesn't
pass...
Yeah, I compiled but tried to run the tests and learned I still need to figure out my setup for make check; then I forgot to make install…
It served its purpose at least.
> @@ -1047,8 +1042,8 @@ pgstat_delete_pending_entry(PgStat_EntryRef *entry_ref)
> void *pending_data = entry_ref->pending;
>
> Assert(pending_data != NULL);
> - /* !fixed_amount stats should be handled explicitly */
> - Assert(!pgstat_kind_info_for(kind)->fixed_amount);
> + /* global stats should be handled explicitly : why?*/
> + Assert(pgstat_kind_info_for(kind)->kind_group == PGSTAT_DYNAMIC);
The pending data infrastructure doesn't provide a way of dealing with fixed
amount stats, and there's no PgStat_EntryRef for them (since they're not in
the hashtable).
Thanks.
David J.
