Hello, I'm a bit concerned that this exposes some of the underlying dshash implementation details to stats kinds. I'd prefer to keep those details encapsulated if possible. That said, considering the performance implications, I can understand the direction taken by this series.
0003: > +bool > +pgstat_drop_current(PgStatShared_HashEntry *shent, dshash_seq_status *hstat) However, pgstat_drop_current() seems too tightly coupled to the state of the underlying dshash scan. shent must be the current entry of exactly the scan represented by hstat, but that relationship is not expressed by the API. As far as I can see, the function only combines two operations: releasing a local entry reference, if any, and calling pgstat_drop_entry_internal() for the current scan entry. Would it be better to keep these as separate operations, perhaps by adding a small helper for releasing the local entry reference, rather than combining them into pgstat_drop_current()? > +bool > +pgstat_has_pending(PgStatShared_HashEntry *shent) I also wonder if this should be added at this point. The name is rather vague, and the function is not used anywhere in this series. Unless there is an immediate caller, I'd be inclined to leave it out for now and add it later if a concrete need arises. Regards, -- Kyotaro Horiguchi NTT Open Source Software Center
