Get rid of GenericXLogUnregister().

This routine is unsafe as implemented, because it invalidates the page
image pointers returned by previous GenericXLogRegister() calls.

Rather than complicate the API or the implementation to avoid that,
let's just get rid of it; the use-case for having it seems much
too thin to justify a lot of work here.

While at it, do some wordsmithing on the SGML docs for generic WAL.

doc/src/sgml/generic-wal.sgml

@@ -1,70 +1,84 @@
 <!-- doc/src/sgml/generic-wal.sgml -->
 
 <chapter id="generic-wal">
- <title>Generic WAL records</title>
+ <title>Generic WAL Records</title>
 
   <para>
-   Despite all built-in access methods and WAL-logged modules having their own
+   Although all built-in WAL-logged modules have their own types of WAL
-   types of WAL records, there is also a generic WAL record type, which describes
+   records, there is also a generic WAL record type, which describes changes
-   changes to pages in a generic way.  This is useful for extensions that
+   to pages in a generic way.  This is useful for extensions that provide
-   provide custom access methods, because they cannot register their own
+   custom access methods, because they cannot register their own WAL redo
-   WAL redo routines.
+   routines.
   </para>
 
   <para>
    The API for constructing generic WAL records is defined in
-   <filename>generic_xlog.h</> and implemented in <filename>generic_xlog.c</>.
+   <filename>access/generic_xlog.h</> and implemented
-   Each generic WAL record must be constructed by following these steps:
+   in <filename>access/transam/generic_xlog.c</>.
+  </para>
+
+  <para>
+   To perform a WAL-logged data update using the generic WAL record
+   facility, follow these steps:
 
    <orderedlist>
     <listitem>
      <para>
       <function>state = GenericXLogStart(relation)</> &mdash; start
-      construction of a generic xlog record for the given relation.
+      construction of a generic WAL record for the given relation.
      </para>
     </listitem>
 
     <listitem>
      <para>
       <function>page = GenericXLogRegister(state, buffer, isNew)</> &mdash;
-      register one or more buffers (one at a time) for the current generic
+      register a buffer to be modified within the current generic WAL
-      xlog record.  This function returns a copy of the page image, where
+      record.  This function returns a pointer to a temporary copy of the
-      modifications can be made.  The second argument indicates if the page
+      buffer's page, where modifications should be made.  (Do not modify the
-      is new (eventually, this will result in a full page image being put into
+      buffer's contents directly.)  The third argument indicates if the page
-      the xlog record).
+      is new; if true, this will result in a full-page image rather than a
+      delta update being included in the WAL record.
+      <function>GenericXLogRegister</> can be repeated if the WAL-logged
+      action needs to modify multiple pages.
      </para>
     </listitem>
 
     <listitem>
      <para>
-      Apply modifications to page images obtained in the previous step.
+      Apply modifications to the page images obtained in the previous step.
      </para>
     </listitem>
 
     <listitem>
      <para>
-      <function>GenericXLogFinish(state)</> &mdash; finish construction of
+      <function>GenericXLogFinish(state)</> &mdash; apply the changes to
-      a generic xlog record.
+      the buffers and emit the generic WAL record.
      </para>
     </listitem>
    </orderedlist>
   </para>
 
   <para>
-   The xlog record construction can be canceled between any of the above
+   WAL record construction can be canceled between any of the above steps by
-   steps by calling <function>GenericXLogAbort(state)</>.  This will discard all
+   calling <function>GenericXLogAbort(state)</>.  This will discard all
    changes to the page image copies.
   </para>
 
   <para>
-   Please note the following points when constructing generic xlog records:
+   Please note the following points when using the generic WAL record
+   facility:
+
    <itemizedlist>
     <listitem>
      <para>
-      No direct modifications of page images are allowed!  All modifications
+      No direct modifications of buffers are allowed!  All modifications
       must be done in copies acquired from <function>GenericXLogRegister()</>.
-      In other words, code which makes generic xlog records must never call
+      In other words, code that makes generic WAL records should never call
-      <function>BufferGetPage()</>.
+      <function>BufferGetPage()</> for itself.  However, it remains the
+      caller's responsibility to pin/unpin and lock/unlock the buffers at
+      appropriate times.  Exclusive lock must be held on each target buffer
+      from before <function>GenericXLogRegister()</> until after
+      <function>GenericXLogFinish()</>.
      </para>
     </listitem>
 
@@ -72,70 +86,71 @@
      <para>
       Registrations of buffers (step 2) and modifications of page images
       (step 3) can be mixed freely, i.e., both steps may be repeated in any
-      sequence.  The only restriction is that you can modify a page image
+      sequence.  Keep in mind that buffers should be registered in the same
-      only after the registration of the corresponding buffer.
+      order in which locks are to be obtained on them during replay.
      </para>
     </listitem>
 
     <listitem>
      <para>
-      After registration, the buffer can also be unregistered by calling
+      The maximum number of buffers that can be registered for a generic WAL
-      <function>GenericXLogUnregister(buffer)</>.  In this case, the changes
+      record is <literal>MAX_GENERIC_XLOG_PAGES</>.  An error will be thrown
-      made to that particular page image copy will be discarded.
+      if this limit is exceeded.
      </para>
     </listitem>
 
     <listitem>
      <para>
-      Generic xlog assumes that pages are using standard layout.  I.e., all
+      Generic WAL assumes that the pages to be modified have standard
-      information between pd_lower and pd_upper will be discarded.
+      layout, and in particular that there is no useful data between
+      <structfield>pd_lower</> and <structfield>pd_upper</>.
      </para>
     </listitem>
 
     <listitem>
      <para>
-      The maximum number of buffers that can be simultaneously registered
+      Since you are modifying copies of buffer
-      for a generic xlog is <literal>MAX_GENERIC_XLOG_PAGES</>.  An error will
+      pages, <function>GenericXLogStart()</> does not start a critical
-      be thrown if this limit is exceeded.
+      section.  Thus, you can safely do memory allocation, error throwing,
+      etc. between <function>GenericXLogStart()</> and
+      <function>GenericXLogFinish()</>.  The only actual critical section is
+      present inside <function>GenericXLogFinish()</>.  There is no need to
+      worry about calling  <function>GenericXLogAbort()</> during an error
+      exit, either.
      </para>
     </listitem>
+
     <listitem>
      <para>
-      Since you modify copies of page images, <function>GenericXLogStart()</>
+      <function>GenericXLogFinish()</> takes care of marking buffers dirty
-      does not start a critical section.  Thus, you can do memory allocation,
-      error throwing, etc. between <function>GenericXLogStart()</> and
-      <function>GenericXLogFinish()</>.  The actual critical section is present
-      inside <function>GenericXLogFinish()</>.
-     </para>
-    </listitem>
-    <listitem>
-     <para>
-      <function>GenericXLogFinish()</> takes care of marking buffers as dirty
       and setting their LSNs.  You do not need to do this explicitly.
      </para>
     </listitem>
+
     <listitem>
      <para>
-      For unlogged relations, everything works the same except there is no
+      For unlogged relations, everything works the same except that no
-      WAL record produced.  Thus, you typically do not need to do any explicit
+      actual WAL record is emitted.  Thus, you typically do not need to do
-      checks for unlogged relations.
+      any explicit checks for unlogged relations.
      </para>
     </listitem>
+
     <listitem>
      <para>
-      If a registered buffer is not new, the generic xlog record contains
+      The generic WAL redo function will acquire exclusive locks to buffers
-      a delta between the old and the new page images.  This delta is produced
-      using per byte comparison.  The current delta mechanism is not effective
-      for moving data within a page and may be improved in the future.
-     </para>
-    </listitem>
-    <listitem>
-     <para>
-      The generic xlog redo function will acquire exclusive locks to buffers
       in the same order as they were registered.  After redoing all changes,
       the locks will be released in the same order.
      </para>
     </listitem>
+
+    <listitem>
+     <para>
+      If a registered buffer is not new, the generic WAL record contains
+      a delta between the old and the new page images.  This delta is based
+      on byte-by-byte comparison.  This is not very compact for the case of
+      moving data within a page, and might be improved in the future.
+     </para>
+    </listitem>
    </itemizedlist>
   </para>
 </chapter>

src/backend/access/transam/generic_xlog.c

@@ -257,35 +257,6 @@ GenericXLogRegister(GenericXLogState *state, Buffer buffer, bool isNew)
 	return NULL;
 }
 
-/*
- * Unregister particular buffer for generic xlog record.
- *
- * XXX this is dangerous and should go away.
- */
-void
-GenericXLogUnregister(GenericXLogState *state, Buffer buffer)
-{
-	int			block_id;
-
-	/* Find block in array to unregister */
-	for (block_id = 0; block_id < MAX_GENERIC_XLOG_PAGES; block_id++)
-	{
-		if (state->pages[block_id].buffer == buffer)
-		{
-			/*
-			 * Preserve order of pages in array because it could matter for
-			 * concurrency.
-			 */
-			memmove(&state->pages[block_id], &state->pages[block_id + 1],
-				 (MAX_GENERIC_XLOG_PAGES - block_id - 1) * sizeof(PageData));
-			state->pages[MAX_GENERIC_XLOG_PAGES - 1].buffer = InvalidBuffer;
-			return;
-		}
-	}
-
-	elog(ERROR, "registered generic xlog buffer not found");
-}
-
 /*
  * Apply changes represented by GenericXLogState to the actual buffers,
  * and emit a generic xlog record.

src/include/access/generic_xlog.h

@@ -30,7 +30,6 @@ typedef struct GenericXLogState GenericXLogState;
 extern GenericXLogState *GenericXLogStart(Relation relation);
 extern Page GenericXLogRegister(GenericXLogState *state, Buffer buffer,
 					bool isNew);
-extern void GenericXLogUnregister(GenericXLogState *state, Buffer buffer);
 extern XLogRecPtr GenericXLogFinish(GenericXLogState *state);
 extern void GenericXLogAbort(GenericXLogState *state);