Re: Pluggable Storage - Andres's take

On Fri, Aug 24, 2018 at 12:50 PM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2018-08-24 11:55:41 +1000, Haribabu Kommi wrote:
> On Tue, Aug 21, 2018 at 6:59 PM Andres Freund <andres@anarazel.de> wrote:
>
> > On 2018-08-21 16:55:47 +1000, Haribabu Kommi wrote:
> > > On Sun, Aug 5, 2018 at 7:48 PM Andres Freund <andres@anarazel.de> wrote:
> > > > I'm currently in the process of rebasing zheap onto the pluggable
> > > > storage work. The goal, which seems to work surprisingly well, is to
> > > > find issues that the current pluggable storage patch doesn't yet deal
> > > > with.  I plan to push a tree including a lot of fixes and improvements
> > > > soon.
> > > >
> > > That's good. Did you find any problems in porting zheap into pluggable
> > > storage? Does it needs any API changes or new API requirement?
> >
> > A lot, yes. The big changes are:
> > - removal of HeapPageScanDesc
> > - introduction of explicit support functions for tablesample & bitmap scans
> > - introduction of callbacks for vacuum_rel, cluster
> >
> > And quite a bit more along those lines.
> >
>
> OK. Those are quite a bit of changes.

I've pushed a current version of that to my git tree to the
pluggable-storage branch. It's not really a version that I think makese
sense to review or such, but it's probably more useful if you work based
on that.  There's also the pluggable-zheap branch, which I found
extremely useful to develop against.

OK. Thanks, will check that also.

 

- fixing your (Haribabu's) slotification of copy patch to compute memory
  usage somehow

I will check it.

 

- table creation callback, currently the pluggable-zheap patch has a few
  conditionals outside of access/zheap for that purpose (see RelationTruncate

I will check it.

 

And then:
- lotsa cleanups
- rebasing onto a newer version of the abstract slot patchset
- splitting out smaller patches


You'd moved the bulk insert into tableam callbacks - I don't quite get
why? There's not really anything AM specific in that code?

The main reason of adding them to AM is just to provide a control to

the specific AM to decide whether they can support the bulk insert or

not?

Current framework doesn't support AM specific bulk insert state to be

passed from one function to another and it's structure is fixed. This needs

to be enhanced to add AM specific private members also.

 

> > > Does the new TupleTableSlot abstraction patches has fixed any of these
> > > issues in the recent thread [1]? so that I can look into the change of
> > > FDW API to return slot instead of tuple.
> >
> > Yea, that'd be a good thing to start with.
> >
>
> I found out only the RefetchForeignRow API needs the change and done the
> same.
> Along with that, I fixed all the issues of  running make check-world.
> Attached patches
> for the same.

Thanks, that's really helpful!  I'll try to merge these soon.

I can share the rebased patches for the fixes, so that it will be easy to merge.