Tomas Vondra wrote:
> On 18.2.2013 16:50, Alvaro Herrera wrote:
> > Also, it seems to me that the new pgstat_db_requested() logic is
> > slightly bogus (in the "inefficient" sense, not the "incorrect" sense):
> > we should be comparing the timestamp of the request vs. what's already
> > on disk instead of blindly returning true if the list is nonempty. If
> > the request is older than the file, we don't need to write anything and
> > can discard the request. For example, suppose that backend A sends a
> > request for a DB; we write the file. If then quickly backend B also
> > requests stats for the same DB, with the current logic we'd go write the
> > file, but perhaps backend B would be fine with the file we already
> > wrote.
>
> Hmmm, you're probably right.
I left it as is for now; I think it warrants revisiting.
> > Another point is that I think there's a better way to handle nonexistant
> > files, instead of having to read the global file and all the DB records
> > to find the one we want. Just try to read the database file, and only
> > if that fails try to read the global file and compare the timestamp (so
> > there might be two timestamps for each DB, one in the global file and
> > one in the DB-specific file. I don't think this is a problem). The
> > point is avoid having to read the global file if possible.
>
> I don't think that's a good idea. Keeping the timestamps at one place is
> a significant simplification, and I don't think it's worth the
> additional complexity. And the overhead is minimal.
>
> So my vote on this change is -1.
Fair enough.
I have pushed it now. Further testing, of course, is always welcome.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
