Tom Lane wrote:
Heikki Linnakangas <[EMAIL PROTECTED]> writes:
My first thought is that the cycle_ctr just adds extra complexity. The canceled-flag really is the key in Takahiro-san's patch, so we don't need the cycle_ctr anymore.

We don't have to have it in the sense of the code not working without it,
but it probably pays for itself by eliminating useless fsyncs.  The
overhead for it in my proposed implementation is darn near zero in the
non-error case.  Also, Takahiro-san mentioned at one point that he was
concerned to avoid useless fsyncs because of some property of the LDC
patch --- I wasn't too clear on what, but maybe he can explain.

Ok. Perhaps we should not use the canceled-flag but just remove the entry from pendingOpsTable like we used to when mdsync_in_progress isn't set. We might otherwise accumulate a lot of canceled entries in the hash table if checkpoint interval is long and relations are created and dropped as part of normal operation.

I think there's one little bug in the patch:

1. AbsorbFsyncRequests is called. A FORGET message is received, and an entry in the hash table is marked as canceled 2. Another relation with the same relfilenode is created. This can happen after OID wrap-around 3. RememberFsyncRequest is called for the new relation. The old entry is still in the hash table, marked with the canceled-flag, so it's not touched.

The fsync request for the new relation is masked by the old canceled entry. The trivial fix is to always clear the flag on step 3:

--- md.c        2007-04-11 08:18:08.000000000 +0100
+++ md.c.new    2007-04-12 09:21:00.000000000 +0100
@@ -1161,9 +1161,9 @@
 &found);
if (!found) /* new entry, so initialize it */
                {
-                       entry->canceled = false;
                        entry->cycle_ctr = mdsync_cycle_ctr;
                }
+               entry->canceled = false;
                /*
* NB: it's intentional that we don't change cycle_ctr if the entry * already exists. The fsync request must be treated as old, even


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
      subscribe-nomail command to [EMAIL PROTECTED] so that your
      message can get through to the mailing list cleanly

Reply via email to