On Wed, Nov 25, 2020 at 8:00 PM Antonin Houska <a...@cybertec.at> wrote: > > Amit Kapila <amit.kapil...@gmail.com> wrote: > > > On Wed, Nov 18, 2020 at 4:03 PM Antonin Houska <a...@cybertec.at> wrote: > > > > > > Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > > On Fri, Nov 13, 2020 at 6:02 PM Antonin Houska <a...@cybertec.at> wrote: > > > > > > > > > > Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > > > > > > On Thu, Nov 12, 2020 at 2:45 PM Antonin Houska <a...@cybertec.at> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > No background undo > > > > > > > ------------------ > > > > > > > > > > > > > > Reduced complexity of the patch seems to be the priority at the > > > > > > > moment. Amit > > > > > > > suggested that cleanup of an orphaned relation file is simple > > > > > > > enough to be > > > > > > > done on foreground and I agree. > > > > > > > > > > > > > > > > > > > Yeah, I think we should try and see if we can make it work but I > > > > > > noticed that there are few places like AbortOutOfAnyTransaction > > > > > > where > > > > > > we have the assumption that undo will be executed in the background. > > > > > > We need to deal with it. > > > > > > > > > > I think this is o.k. if we always check for unapplied undo during > > > > > startup. > > > > > > > > > > > > > Hmm, how it is ok to leave undo (and rely on startup) unless it is a > > > > PANIC error. IIRC, this path is invoked in non-panic errors as well. > > > > Basically, we won't be able to discard such an undo which doesn't seem > > > > like a good idea. > > > > > > Since failure to apply leaves unconsistent data, I assume it should always > > > cause PANIC, shouldn't it? > > > > > > > But how can we ensure that AbortOutOfAnyTransaction will be called > > only in that scenario? > > I meant that AbortOutOfAnyTransaction should PANIC itself if it sees that > there is unapplied undo, so nothing changes for its callers. Do I still miss > something? >
Adding PANIC in some generic code-path sounds scary. Why can't we simply try to execute undo? > > > > Another thing is that it seems we need to connect to the database to > > > > perform > > > > it which might appear a bit odd that we don't allow users to connect to > > > > the > > > > database but internally we are connecting it. > > > > > > I think the implementation will need to follow the outcome of the part of > > > the > > > discussion that starts at [2], but I see your concern. I'm thinking why > > > database connection is not needed to apply WAL but is needed for UNDO. I > > > think > > > locks make the difference. > > > > > > > Yeah, it would be probably a good idea to see if we can make undo > > apply work without db-connection especially if we want to do before > > allowing connections. The other possibility could be to let discard > > worker do this work lazily after allowing connections. > > Actually I hit the problem of missing connection when playing with the > "undoxacttest" module. Those tests use table_open() / table_close() functions, > but it might not be necessary for the real RMGRs. > How can we apply the action on a page without opening the relation? -- With Regards, Amit Kapila.
