On Fri, Dec 6, 2019 at 5:00 PM Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>
> On Fri, 6 Dec 2019 at 15:40, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> >
> > Few comments:
> > ----------------------
> >
> > 1.
> > + /* Now that the state fields are initialized, it is safe to return it. */
> > + *iter_state = state;
> > +
> > /* allocate heap */
> > state->heap =
> > binaryheap_allocate(state->nr_txns,
> > ReorderBufferIterCompare,
> >
> > Is there a reason for not initializing iter_state after
> > binaryheap_allocate? If we do so, then we don't need additional check
> > you have added in ReorderBufferIterTXNFinish.
>
> If iter_state is initialized *after* binaryheap_allocate, then we
> won't be able to close the vfds if binaryheap_allocate() ereports().
>
Is it possible to have vfds opened before binaryheap_allocate(), if so how?
> >
> > 5. One naive question about the usage of PathNameOpenFile(). When it
> > reaches the max limit, it will automatically close one of the files,
> > but how will that be reflected in the data structure (TXNEntryFile)
> > you are managing. Basically, when PathNameOpenFile closes some file,
> > how will the corresponding vfd in TXNEntryFile be changed. Because if
> > it is not changed, then won't it start pointing to some wrong
> > filehandle?
>
> In PathNameOpenFile(), excess kernel fds could be closed
> (ReleaseLruFiles). But with that, the vfds themselves don't get
> invalidated. Only the underlying kernel fd gets closed, and the
> vfd->fd is marked VFD_CLOSED. The vfd array element remains valid (a
> non-null vfd->fileName means the vfd slot is valid; check
> FileIsValid). So later, when FileRead(vfd1) is called and that vfd1
> happens to be the one that had got it's kernel fd closed, it gets
> opened again through FileAccess().
>
I was under impression that once the fd is closed due to excess kernel
fds that are opened, the slot in VfdCache array could be resued by
someone else, but on closer inspection that is not true. It will be
only available for reuse after we explicitly call FileClose, right?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com