From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | "Brightwell, Adam" <adam(dot)brightwell(at)crunchydatasolutions(dot)com> |
Cc: | Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [TODO] Track number of files ready to be archived in pg_stat_archiver |
Date: | 2014-10-21 10:23:00 |
Message-ID: | CAOBaU_YiLuQ+me+tk1ehK9rPp6B7WJhCN-utQNvt_2ekq_Y9uw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Oct 21, 2014 at 7:35 AM, Brightwell, Adam <
adam(dot)brightwell(at)crunchydatasolutions(dot)com> wrote:
> Julien,
>
> The following is an initial review:
>
>
Thanks for the review.
> * Applies cleanly to master (f330a6d).
> * Regression tests updated and pass, including 'check-world'.
> * Documentation updated and builds successfully.
> * Might want to consider replacing the following magic number with a
> constant or perhaps calculated value.
>
> + int basenamelen = (int) strlen(rlde->d_name) - 6;
>
> * Wouldn't it be easier, or perhaps more reliable to use "strrchr()" with
> the following instead?
>
> + strcmp(rlde->d_name + basenamelen, ".ready") == 0)
>
> char *extension = strrchr(ride->d_name, '.');
> ...
> strcmp(extension, ".ready") == 0)
>
> I think this approach might also help to resolve the magic number above.
> For example:
>
> char *extension = strrchr(ride->d_name, '.');
> int basenamelen = (int) strlen(ride->d_name) - strlen(extension);
>
>
Actually, I used the same loop as the archiver one (see
backend/postmaster/pgarch.c, function pgarch_readyXlog) to get the exact
same number of files.
If we change it in this patch, it would be better to change it everywhere.
What do you think ?
-Adam
>
> --
> Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
> Database Engineer - www.crunchydatasolutions.com
>
From | Date | Subject | |
---|---|---|---|
Next Message | Florian Pflug | 2014-10-21 13:14:00 | Re: Patch: Add launchd Support |
Previous Message | Marti Raudsepp | 2014-10-21 10:00:35 | Re: [PATCH] Simplify EXISTS subqueries containing LIMIT |