Re: unnecessary creation of FSM files during bootstrap mode
Amit Kapila writes: > On Fri, Jan 11, 2019 at 5:00 AM Tom Lane wrote: >> It's also possible that you just aren't exercising the cases where >> trouble occurs. In particular, noting this bit in InsertOneValue(): >> OidOutputFunctionCall(typoutput, values[i]; > I have tried initdb with --debug option (If I am not wrong, it runs > initdb under DEBUG5 mode) and didn't hit any problem after applying > the patch. Are you expecting that we might try to open pg_proc at > that place which can lead to the problem? Yes, I was concerned about regprocout in particular. It might be okay as long as we don't try to add any regproc columns to the BKI_BOOTSTRAP catalogs. regards, tom lane
Re: unnecessary creation of FSM files during bootstrap mode
On Fri, Jan 11, 2019 at 5:00 AM Tom Lane wrote:
> It's also possible that you just aren't exercising the cases where
> trouble occurs. In particular, noting this bit in InsertOneValue():
>
> /*
> * We use ereport not elog here so that parameters aren't evaluated unless
> * the message is going to be printed, which generally it isn't
> */
> ereport(DEBUG4,
> (errmsg_internal("inserted -> %s",
> OidOutputFunctionCall(typoutput, values[i];
>
> I'd counsel running initdb under DEBUG4 or higher before deciding
> you're out of the woods.
>
I have tried initdb with --debug option (If I am not wrong, it runs
initdb under DEBUG5 mode) and didn't hit any problem after applying
the patch. Are you expecting that we might try to open pg_proc at
that place which can lead to the problem?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Re: unnecessary creation of FSM files during bootstrap mode
John Naylor writes:
> Thought I'd ping...
Sorry, I'd not been paying attention to this thread.
> On Sat, Dec 15, 2018 at 12:02 AM Amit Kapila wrote:
>> As pointed out by John Naylor [1], it seems during bootstrap mode, we
>> are always creating FSM files which are not required. In commit's
>> b9d01fe288 and 3908473c80, we have added some code where we allowed
>> the creation of files during mdopen even if they didn't exist during
>> the bootstrap mode. The comments in the code say: "During bootstrap,
>> there are cases where a system relation will be accessed (by internal
>> backend processes) before the bootstrap script nominally creates it."
>> I am sure this will be the case when that code is added but is it
>> required today?
I'm surprised to hear that it isn't, but if we get through initdb
then it must not be. The special case was certainly still necessary
as of 3908473c80, for the BKI_BOOTSTRAP catalogs. It didn't bother me
at the time that those were special --- how are you going to create
pg_class, in particular, without cheating like mad? But I guess we must
have cleaned up something in higher levels of bootstrapping to the point
where those do get mdcreate'd in advance of being opened.
It's also possible that you just aren't exercising the cases where
trouble occurs. In particular, noting this bit in InsertOneValue():
/*
* We use ereport not elog here so that parameters aren't evaluated unless
* the message is going to be printed, which generally it isn't
*/
ereport(DEBUG4,
(errmsg_internal("inserted -> %s",
OidOutputFunctionCall(typoutput, values[i];
I'd counsel running initdb under DEBUG4 or higher before deciding
you're out of the woods.
(If that does turn out to be a problem, and it's the only problem,
we could discuss whether we really need that debug message at all.)
regards, tom lane
Re: unnecessary creation of FSM files during bootstrap mode
Thought I'd ping... (sorry for the top post) On Sat, Dec 15, 2018 at 12:02 AM Amit Kapila wrote: > > As pointed out by John Naylor [1], it seems during bootstrap mode, we > are always creating FSM files which are not required. In commit's > b9d01fe288 and 3908473c80, we have added some code where we allowed > the creation of files during mdopen even if they didn't exist during > the bootstrap mode. The comments in the code say: "During bootstrap, > there are cases where a system relation will be accessed (by internal > backend processes) before the bootstrap script nominally creates it." > I am sure this will be the case when that code is added but is it > required today? While going through commit 3908473c80, I came across > below comment: > > - * During bootstrap processing, we skip that check, because pg_time, > - * pg_variable, and pg_log get created before their .bki file entries > - * are processed. > - */ > + fd = FileNameOpenFile(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, 0600); > > The system tables mentioned in above commit are not present today, so > do we really need that code and even if it is required shall we do it > only for 'main' or 'init' forks? > > Tom, as you are a committer of the commits b9d01fe288 and 3908473c80, > do you remember anything in this regard? > > > [1] - > https://www.postgresql.org/message-id/CAJVSVGVtf%2B-2sQGVyEZJQh15dpVicpFA6BBiPLugaD4oBEaiHg%40mail.gmail.com > > -- > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com >
unnecessary creation of FSM files during bootstrap mode
As pointed out by John Naylor [1], it seems during bootstrap mode, we are always creating FSM files which are not required. In commit's b9d01fe288 and 3908473c80, we have added some code where we allowed the creation of files during mdopen even if they didn't exist during the bootstrap mode. The comments in the code say: "During bootstrap, there are cases where a system relation will be accessed (by internal backend processes) before the bootstrap script nominally creates it." I am sure this will be the case when that code is added but is it required today? While going through commit 3908473c80, I came across below comment: - * During bootstrap processing, we skip that check, because pg_time, - * pg_variable, and pg_log get created before their .bki file entries - * are processed. - */ + fd = FileNameOpenFile(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, 0600); The system tables mentioned in above commit are not present today, so do we really need that code and even if it is required shall we do it only for 'main' or 'init' forks? Tom, as you are a committer of the commits b9d01fe288 and 3908473c80, do you remember anything in this regard? [1] - https://www.postgresql.org/message-id/CAJVSVGVtf%2B-2sQGVyEZJQh15dpVicpFA6BBiPLugaD4oBEaiHg%40mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
