Hi, On 2015-05-11 16:57:08 +0530, Abhijit Menon-Sen wrote: > I've attached an updated patch for pgstatbloat, as well as a patch to > replace two uses of BuildTupleFromCStrings() elsewhere in pgstattuple.
TBH, I'd rather not touch unrelated things right now. We're pretty badly behind... > I've made the changes you mentioned in your earlier mail, except that I > have not changed the name pending further suggestions about what would > be the best name. I don't really care how it's named, as long as it makes clear that it's not an exact measurement. > > > I haven't checked, but I'm not sure that it's safe/meaningful to > > > call PageGetHeapFreeSpace() on a new page. > > > > OK, I'll check and fix if necessary. > > You're right, PageGetHeapFreeSpace() isn't safe on a new page. I've > added a guard to that call in the attached patch, but I'm not sure > that's the right thing to do. > Should I copy the orphaned new-page handling from lazy_scan_heap? What > about for empty pages? Neither feels like a very good thing to do, but > then neither does skipping the new page silently. Should I count the > space it would have free if it were initialised, but leave the page > alone for VACUUM to deal with? Or just leave it as it is? I think there's absolutely no need for pgstattuple to do anything here. It's not even desirable. > + LockBuffer(buf, BUFFER_LOCK_SHARE); > + > + page = BufferGetPage(buf); > + > + if (!PageIsNew(page)) > + stat->free_space += PageGetHeapFreeSpace(page); > + > + if (PageIsNew(page) || PageIsEmpty(page)) > + { > + UnlockReleaseBuffer(buf); > + continue; > + } Shouldn't new pages be counted as being fully free or at least bloat? Just disregarding them seems wrong. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers