From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: making relfilenodes 56 bits |
Date: | 2022-07-14 11:48:32 |
Message-ID: | CAFiTN-ttOXLX75k_WzRo9ar=VvxFhrHi+rJxns997F+yvkm==A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jul 13, 2022 at 9:35 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Tue, Jul 12, 2022 at 7:21 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >
>
> > In this version, I also removed the struct padding, changed the limit
> > on the number of entries to a nice round 64, and made some comment
> > updates. I considered trying to go further and actually make the file
> > variable-size, so that we never again need to worry about the limit on
> > the number of entries, but I don't actually think that's a good idea.
> > It would require substantially more changes to the code in this file,
> > and that means there's more risk of introducing bugs, and I don't see
> > that there's much value anyway, because if we ever do hit the current
> > limit, we can just raise the limit.
> >
> > If we were going to split up durable_rename(), the only intelligible
> > split I can see would be to have a second version of the function, or
> > a flag to the existing function, that caters to the situation where
> > the old file is already known to have been fsync()'d.
>
> The patch looks good except one minor comment
>
> + * corruption. Since the file might be more tha none standard-size disk
> + * sector in size, we cannot rely on overwrite-in-place. Instead, we generate
>
> typo "more tha none" -> "more than one"
>
I have fixed this and included this change in the new patch series.
Apart from this I have fixed all the pending issues that includes
- Change existing macros to inline functions done in 0001.
- Change pg_class index from (tbspc, relfilenode) to relfilenode and
also change RelidByRelfilenumber(). In RelidByRelfilenumber I have
changed the hash to maintain based on just the relfilenumber but we
still need to pass the tablespace to identify whether it is a shared
relation or not. If we want we can make it bool but I don't think
that is really needed here.
- Changed logic of GetNewRelFileNumber() based on what Robert
described, and instead of tracking the pending logged relnumbercount
now I am tracking last loggedRelNumber, which help little bit in
SetNextRelFileNumber in making code cleaner, but otherwise it doesn't
make much difference.
- Some new asserts in buf_internal inline function to validate value
of computed/input relfilenumber.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v9-0001-Convert-buf_internal.h-macros-to-static-inline-fu.patch | text/x-patch | 12.2 KB |
v9-0002-Preliminary-refactoring-for-supporting-larger-rel.patch | text/x-patch | 23.1 KB |
v9-0003-Remove-the-restriction-that-the-relmap-must-be-51.patch | text/x-patch | 8.4 KB |
v9-0004-Widen-relfilenumber-from-32-bits-to-56-bits.patch | text/x-patch | 91.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2022-07-14 11:51:03 | Re: enable/disable broken for statement triggers on partitioned tables |
Previous Message | Sergey Dudoladov | 2022-07-14 11:20:58 | Re: Introduce "log_connection_stages" setting. |