ovsdb-idl: Improve documentation.
authorBen Pfaff <blp@nicira.com>
Thu, 12 Apr 2012 15:27:56 +0000 (08:27 -0700)
committerBen Pfaff <blp@nicira.com>
Thu, 12 Apr 2012 15:28:14 +0000 (08:28 -0700)
Signed-off-by: Ben Pfaff <blp@nicira.com>
lib/ovsdb-idl.c
lib/ovsdb-idl.h
python/ovs/db/idl.py

index dcbad1054153e1877f2acdf011c5d2a3cd3a3776..bb7da668aebce0f7478f346018ef037229354388 100644 (file)
@@ -357,8 +357,23 @@ ovsdb_idl_wait(struct ovsdb_idl *idl)
     jsonrpc_session_recv_wait(idl->session);
 }
 
     jsonrpc_session_recv_wait(idl->session);
 }
 
-/* Returns a number that represents the state of 'idl'.  When 'idl' is updated
- * (by ovsdb_idl_run()), the return value changes. */
+/* Returns a "sequence number" that represents the state of 'idl'.  When
+ * ovsdb_idl_run() changes the database, the sequence number changes.  The
+ * initial fetch of the entire contents of the remote database is considered to
+ * be one kind of change.  Successfully acquiring a lock, if one has been
+ * configured with ovsdb_idl_set_lock(), is also considered to be a change.
+ *
+ * As long as the sequence number does not change, the client may continue to
+ * use any data structures it obtains from 'idl'.  But when it changes, the
+ * client must not access any of these data structures again, because they
+ * could have freed or reused for other purposes.
+ *
+ * The sequence number can occasionally change even if the database does not.
+ * This happens if the connection to the database drops and reconnects, which
+ * causes the database contents to be reloaded even if they didn't change.  (It
+ * could also happen if the database server sends out a "change" that reflects
+ * what the IDL already thought was in the database.  The database server is
+ * not supposed to do that, but bugs could in theory cause it to do so.) */
 unsigned int
 ovsdb_idl_get_seqno(const struct ovsdb_idl *idl)
 {
 unsigned int
 ovsdb_idl_get_seqno(const struct ovsdb_idl *idl)
 {
@@ -992,6 +1007,7 @@ ovsdb_idl_table_from_class(const struct ovsdb_idl *idl,
     return &idl->tables[table_class - idl->class->tables];
 }
 
     return &idl->tables[table_class - idl->class->tables];
 }
 
+/* Called by ovsdb-idlc generated code. */
 struct ovsdb_idl_row *
 ovsdb_idl_get_row_arc(struct ovsdb_idl_row *src,
                       struct ovsdb_idl_table_class *dst_table_class,
 struct ovsdb_idl_row *
 ovsdb_idl_get_row_arc(struct ovsdb_idl_row *src,
                       struct ovsdb_idl_table_class *dst_table_class,
@@ -1036,6 +1052,8 @@ ovsdb_idl_get_row_arc(struct ovsdb_idl_row *src,
     }
 }
 
     }
 }
 
+/* Searches 'tc''s table in 'idl' for a row with UUID 'uuid'.  Returns a
+ * pointer to the row if there is one, otherwise a null pointer.  */
 const struct ovsdb_idl_row *
 ovsdb_idl_get_row_for_uuid(const struct ovsdb_idl *idl,
                            const struct ovsdb_idl_table_class *tc,
 const struct ovsdb_idl_row *
 ovsdb_idl_get_row_for_uuid(const struct ovsdb_idl *idl,
                            const struct ovsdb_idl_table_class *tc,
@@ -1058,6 +1076,12 @@ next_real_row(struct ovsdb_idl_table *table, struct hmap_node *node)
     return NULL;
 }
 
     return NULL;
 }
 
+/* Returns a row in 'table_class''s table in 'idl', or a null pointer if that
+ * table is empty.
+ *
+ * Database tables are internally maintained as hash tables, so adding or
+ * removing rows while traversing the same table can cause some rows to be
+ * visited twice or not at apply. */
 const struct ovsdb_idl_row *
 ovsdb_idl_first_row(const struct ovsdb_idl *idl,
                     const struct ovsdb_idl_table_class *table_class)
 const struct ovsdb_idl_row *
 ovsdb_idl_first_row(const struct ovsdb_idl *idl,
                     const struct ovsdb_idl_table_class *table_class)
@@ -1067,6 +1091,8 @@ ovsdb_idl_first_row(const struct ovsdb_idl *idl,
     return next_real_row(table, hmap_first(&table->rows));
 }
 
     return next_real_row(table, hmap_first(&table->rows));
 }
 
+/* Returns a row following 'row' within its table, or a null pointer if 'row'
+ * is the last row in its table. */
 const struct ovsdb_idl_row *
 ovsdb_idl_next_row(const struct ovsdb_idl_row *row)
 {
 const struct ovsdb_idl_row *
 ovsdb_idl_next_row(const struct ovsdb_idl_row *row)
 {
@@ -1143,6 +1169,11 @@ ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *row)
 static void ovsdb_idl_txn_complete(struct ovsdb_idl_txn *txn,
                                    enum ovsdb_idl_txn_status);
 
 static void ovsdb_idl_txn_complete(struct ovsdb_idl_txn *txn,
                                    enum ovsdb_idl_txn_status);
 
+/* Returns a string representation of 'status'.  The caller must not modify or
+ * free the returned string.
+ *
+ * The return value is probably useful only for debug log messages and unit
+ * tests. */
 const char *
 ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
 {
 const char *
 ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
 {
@@ -1167,6 +1198,9 @@ ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
     return "<unknown>";
 }
 
     return "<unknown>";
 }
 
+/* Starts a new transaction on 'idl'.  A given ovsdb_idl may only have a single
+ * active transaction at a time.  See the large comment in ovsdb-idl.h for
+ * general information on transactions. */
 struct ovsdb_idl_txn *
 ovsdb_idl_txn_create(struct ovsdb_idl *idl)
 {
 struct ovsdb_idl_txn *
 ovsdb_idl_txn_create(struct ovsdb_idl *idl)
 {
@@ -1209,6 +1243,13 @@ ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *txn, const char *s, ...)
     va_end(args);
 }
 
     va_end(args);
 }
 
+/* Marks 'txn' as a transaction that will not actually modify the database.  In
+ * almost every way, the transaction is treated like other transactions.  It
+ * must be committed or aborted like other transactions, it will be sent to the
+ * database server like other transactions, and so on.  The only difference is
+ * that the operations sent to the database server will include, as the last
+ * step, an "abort" operation, so that any changes made by the transaction will
+ * not actually take effect. */
 void
 ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
 {
 void
 ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
 {
@@ -1241,6 +1282,11 @@ ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn,
     txn->inc_row = row->uuid;
 }
 
     txn->inc_row = row->uuid;
 }
 
+/* Destroys 'txn' and frees all associated memory.  If ovsdb_idl_txn_commit()
+ * has been called for 'txn' but the commit is still incomplete (that is, the
+ * last call returned TXN_INCOMPLETE) then the transaction may or may not still
+ * end up committing at the database server, but the client will not be able to
+ * get any further status information back. */
 void
 ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
 {
 void
 ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
 {
@@ -1260,6 +1306,7 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
     free(txn);
 }
 
     free(txn);
 }
 
+/* Causes poll_block() to wake up if 'txn' has completed committing. */
 void
 ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *txn)
 {
 void
 ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *txn)
 {
@@ -1390,6 +1437,55 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn *txn)
     hmap_init(&txn->txn_rows);
 }
 
     hmap_init(&txn->txn_rows);
 }
 
+/* Attempts to commit 'txn'.  Returns the status of the commit operation, one
+ * of the following TXN_* constants:
+ *
+ *   TXN_INCOMPLETE:
+ *
+ *       The transaction is in progress, but not yet complete.  The caller
+ *       should call again later, after calling ovsdb_idl_run() to let the IDL
+ *       do OVSDB protocol processing.
+ *
+ *   TXN_UNCHANGED:
+ *
+ *       The transaction is complete.  (It didn't actually change the database,
+ *       so the IDL didn't send any request to the database server.)
+ *
+ *   TXN_ABORTED:
+ *
+ *       The caller previously called ovsdb_idl_txn_abort().
+ *
+ *   TXN_SUCCESS:
+ *
+ *       The transaction was successful.  The update made by the transaction
+ *       (and possibly other changes made by other database clients) should
+ *       already be visible in the IDL.
+ *
+ *   TXN_TRY_AGAIN:
+ *
+ *       The transaction failed for some transient reason, e.g. because a
+ *       "verify" operation reported an inconsistency or due to a network
+ *       problem.  The caller should wait for a change to the database, then
+ *       compose a new transaction, and commit the new transaction.
+ *
+ *       Use the return value of ovsdb_idl_get_seqno() to wait for a change in
+ *       the database.  It is important to use its return value *before* the
+ *       initial call to ovsdb_idl_txn_commit() as the baseline for this
+ *       purpose, because the change that one should wait for can happen after
+ *       the initial call but before the call that returns TXN_TRY_AGAIN, and
+ *       using some other baseline value in that situation could cause an
+ *       indefinite wait if the database rarely changes.
+ *
+ *   TXN_NOT_LOCKED:
+ *
+ *       The transaction failed because the IDL has been configured to require
+ *       a database lock (with ovsdb_idl_set_lock()) but didn't get it yet or
+ *       has already lost it.
+ *
+ * Committing a transaction rolls back all of the changes that it made to the
+ * IDL's copy of the database.  If the transaction commits successfully, then
+ * the database server will send an update and, thus, the IDL will be updated
+ * with the committed changes. */
 enum ovsdb_idl_txn_status
 ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
 {
 enum ovsdb_idl_txn_status
 ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
 {
@@ -1595,7 +1691,10 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
 
 /* Attempts to commit 'txn', blocking until the commit either succeeds or
  * fails.  Returns the final commit status, which may be any TXN_* value other
 
 /* Attempts to commit 'txn', blocking until the commit either succeeds or
  * fails.  Returns the final commit status, which may be any TXN_* value other
- * than TXN_INCOMPLETE. */
+ * than TXN_INCOMPLETE.
+ *
+ * This function calls ovsdb_idl_run() on 'txn''s IDL, so it may cause the
+ * return value of ovsdb_idl_get_seqno() to change. */
 enum ovsdb_idl_txn_status
 ovsdb_idl_txn_commit_block(struct ovsdb_idl_txn *txn)
 {
 enum ovsdb_idl_txn_status
 ovsdb_idl_txn_commit_block(struct ovsdb_idl_txn *txn)
 {
@@ -1611,6 +1710,9 @@ ovsdb_idl_txn_commit_block(struct ovsdb_idl_txn *txn)
     return status;
 }
 
     return status;
 }
 
+/* Returns the final (incremented) value of the column in 'txn' that was set to
+ * be incremented by ovsdb_idl_txn_increment().  'txn' must have committed
+ * successfully. */
 int64_t
 ovsdb_idl_txn_get_increment_new_value(const struct ovsdb_idl_txn *txn)
 {
 int64_t
 ovsdb_idl_txn_get_increment_new_value(const struct ovsdb_idl_txn *txn)
 {
@@ -1618,6 +1720,12 @@ ovsdb_idl_txn_get_increment_new_value(const struct ovsdb_idl_txn *txn)
     return txn->inc_new_value;
 }
 
     return txn->inc_new_value;
 }
 
+/* Aborts 'txn' without sending it to the database server.  This is effective
+ * only if ovsdb_idl_txn_commit() has not yet been called for 'txn'.
+ * Otherwise, it has no effect.
+ *
+ * Aborting a transaction doesn't free its memory.  Use
+ * ovsdb_idl_txn_destroy() to do that. */
 void
 ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
 {
 void
 ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
 {
@@ -1627,6 +1735,14 @@ ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
     }
 }
 
     }
 }
 
+/* Returns a string that reports the error status for 'txn'.  The caller must
+ * not modify or free the returned string.  A call to ovsdb_idl_txn_destroy()
+ * for 'txn' may free the returned string.
+ *
+ * The return value is ordinarily one of the strings that
+ * ovsdb_idl_txn_status_to_string() would return, but if the transaction failed
+ * due to an error reported by the database server, the return value is that
+ * error. */
 const char *
 ovsdb_idl_txn_get_error(const struct ovsdb_idl_txn *txn)
 {
 const char *
 ovsdb_idl_txn_get_error(const struct ovsdb_idl_txn *txn)
 {
@@ -2101,6 +2217,8 @@ ovsdb_idl_txn_process_reply(struct ovsdb_idl *idl,
     return true;
 }
 
     return true;
 }
 
+/* Returns the transaction currently active for 'row''s IDL.  A transaction
+ * must currently be active. */
 struct ovsdb_idl_txn *
 ovsdb_idl_txn_get(const struct ovsdb_idl_row *row)
 {
 struct ovsdb_idl_txn *
 ovsdb_idl_txn_get(const struct ovsdb_idl_row *row)
 {
@@ -2109,6 +2227,7 @@ ovsdb_idl_txn_get(const struct ovsdb_idl_row *row)
     return txn;
 }
 
     return txn;
 }
 
+/* Returns the IDL on which 'txn' acts. */
 struct ovsdb_idl *
 ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *txn)
 {
 struct ovsdb_idl *
 ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *txn)
 {
index 34ccf061931cb7542f56a2e7e7f72f551347d2aa..33cd3401b4dbf91751e1d391f9f915538bd651f4 100644 (file)
@@ -112,7 +112,56 @@ const struct ovsdb_datum *ovsdb_idl_get(const struct ovsdb_idl_row *,
 
 bool ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *);
 \f
 
 bool ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *);
 \f
-/* Transactions. */
+/* Transactions.
+ *
+ * A transaction may modify the contents of a database by modifying the values
+ * of columns, deleting rows, inserting rows, or adding checks that columns in
+ * the database have not changed ("verify" operations), through
+ * ovsdb_idl_txn_*() functions.  (The OVSDB IDL code generator produces helper
+ * functions that internally call the ovsdb_idl_txn_*() functions.  These are
+ * likely to be more convenient.)
+ *
+ * Reading and writing columns and inserting and deleting rows are all
+ * straightforward.  The reasons to verify columns are less obvious.
+ * Verification is the key to maintaining transactional integrity.  Because
+ * OVSDB handles multiple clients, it can happen that between the time that
+ * OVSDB client A reads a column and writes a new value, OVSDB client B has
+ * written that column.  Client A's write should not ordinarily overwrite
+ * client B's, especially if the column in question is a "map" column that
+ * contains several more or less independent data items.  If client A adds a
+ * "verify" operation before it writes the column, then the transaction fails
+ * in case client B modifies it first.  Client A will then see the new value of
+ * the column and compose a new transaction based on the new contents written
+ * by client B.
+ *
+ * When a transaction is complete, which must be before the next call to
+ * ovsdb_idl_run() on 'idl', call ovsdb_idl_txn_commit() or
+ * ovsdb_idl_txn_abort().
+ *
+ * The life-cycle of a transaction looks like this:
+ *
+ * 1. Create the transaction and record the initial sequence number:
+ *
+ *     seqno = ovsdb_idl_get_seqno(idl);
+ *     txn = ovsdb_idl_txn_create(idl);
+ *
+ * 2. Modify the database with ovsdb_idl_txn_*() functions directly or
+ *    indirectly.
+ *
+ * 3. Commit the transaction by calling ovsdb_idl_txn_commit().  The first call
+ *    to this function probably returns TXN_INCOMPLETE.  The client must keep
+ *    calling again along as this remains true, calling ovsdb_idl_run() in
+ *    between to let the IDL do protocol processing.  (If the client doesn't
+ *    have anything else to do in the meantime, it can use
+ *    ovsdb_idl_txn_commit_block() to avoid having to loop itself.)
+ *
+ * 4. If the final status is TXN_TRY_AGAIN, wait for ovsdb_idl_get_seqno() to
+ *    change from the saved 'seqno' (it's possible that it's already changed,
+ *    in which case the client should not wait at all), then start over from
+ *    step 1.  Only a call to ovsdb_idl_run() will change the return value of
+ *    ovsdb_idl_get_seqno().  (ovsdb_idl_txn_commit_block() calls
+ *    ovsdb_idl_run().)
+ */
 
 enum ovsdb_idl_txn_status {
     TXN_UNCOMMITTED,            /* Not yet committed or aborted. */
 
 enum ovsdb_idl_txn_status {
     TXN_UNCOMMITTED,            /* Not yet committed or aborted. */
index fba20dcb22b56d56661bbc2e18404592a4f152cd..36a7bab285efa06063def07b4327c0ecda240ece 100644 (file)
@@ -52,7 +52,13 @@ class Idl:
       'rows' map values.  Refer to Row for more details.
 
     - 'change_seqno': A number that represents the IDL's state.  When the IDL
       'rows' map values.  Refer to Row for more details.
 
     - 'change_seqno': A number that represents the IDL's state.  When the IDL
-      is updated (by Idl.run()), its value changes.
+      is updated (by Idl.run()), its value changes.  The sequence number can
+      occasionally change even if the database does not.  This happens if the
+      connection to the database drops and reconnects, which causes the
+      database contents to be reloaded even if they didn't change.  (It could
+      also happen if the database server sends out a "change" that reflects
+      what the IDL already thought was in the database.  The database server is
+      not supposed to do that, but bugs could in theory cause it to do so.)
 
     - 'lock_name': The name of the lock configured with Idl.set_lock(), or None
       if no lock is configured.
 
     - 'lock_name': The name of the lock configured with Idl.set_lock(), or None
       if no lock is configured.
@@ -635,6 +641,49 @@ class _InsertedRow(object):
 
 
 class Transaction(object):
 
 
 class Transaction(object):
+    """A transaction may modify the contents of a database by modifying the
+    values of columns, deleting rows, inserting rows, or adding checks that
+    columns in the database have not changed ("verify" operations), through
+    Row methods.
+
+    Reading and writing columns and inserting and deleting rows are all
+    straightforward.  The reasons to verify columns are less obvious.
+    Verification is the key to maintaining transactional integrity.  Because
+    OVSDB handles multiple clients, it can happen that between the time that
+    OVSDB client A reads a column and writes a new value, OVSDB client B has
+    written that column.  Client A's write should not ordinarily overwrite
+    client B's, especially if the column in question is a "map" column that
+    contains several more or less independent data items.  If client A adds a
+    "verify" operation before it writes the column, then the transaction fails
+    in case client B modifies it first.  Client A will then see the new value
+    of the column and compose a new transaction based on the new contents
+    written by client B.
+
+    When a transaction is complete, which must be before the next call to
+    Idl.run(), call Transaction.commit() or Transaction.abort().
+
+    The life-cycle of a transaction looks like this:
+
+    1. Create the transaction and record the initial sequence number:
+
+        seqno = idl.change_seqno(idl)
+        txn = Transaction(idl)
+
+    2. Modify the database with Row and Transaction methods.
+
+    3. Commit the transaction by calling Transaction.commit().  The first call
+       to this function probably returns Transaction.INCOMPLETE.  The client
+       must keep calling again along as this remains true, calling Idl.run() in
+       between to let the IDL do protocol processing.  (If the client doesn't
+       have anything else to do in the meantime, it can use
+       Transaction.commit_block() to avoid having to loop itself.)
+
+    4. If the final status is Transaction.TRY_AGAIN, wait for Idl.change_seqno
+       to change from the saved 'seqno' (it's possible that it's already
+       changed, in which case the client should not wait at all), then start
+       over from step 1.  Only a call to Idl.run() will change the return value
+       of Idl.change_seqno.  (Transaction.commit_block() calls Idl.run().)"""
+
     # Status values that Transaction.commit() can return.
     UNCOMMITTED = "uncommitted"  # Not yet committed or aborted.
     UNCHANGED = "unchanged"      # Transaction didn't include any changes.
     # Status values that Transaction.commit() can return.
     UNCOMMITTED = "uncommitted"  # Not yet committed or aborted.
     UNCHANGED = "unchanged"      # Transaction didn't include any changes.
@@ -694,6 +743,8 @@ class Transaction(object):
         self._comments.append(comment)
 
     def wait(self, poller):
         self._comments.append(comment)
 
     def wait(self, poller):
+        """Causes poll_block() to wake up if this transaction has completed
+        committing."""
         if self._status not in (Transaction.UNCOMMITTED,
                                 Transaction.INCOMPLETE):
             poller.immediate_wake()
         if self._status not in (Transaction.UNCOMMITTED,
                                 Transaction.INCOMPLETE):
             poller.immediate_wake()
@@ -722,16 +773,56 @@ class Transaction(object):
         self._txn_rows = {}
 
     def commit(self):
         self._txn_rows = {}
 
     def commit(self):
-        """Attempts to commit this transaction and returns the status of the
-        commit operation, one of the constants declared as class attributes.
-        If the return value is Transaction.INCOMPLETE, then the transaction is
-        not yet complete and the caller should try calling again later, after
-        calling Idl.run() to run the Idl.
+        """Attempts to commit 'txn'.  Returns the status of the commit
+        operation, one of the following constants:
+
+          Transaction.INCOMPLETE:
+
+              The transaction is in progress, but not yet complete.  The caller
+              should call again later, after calling Idl.run() to let the
+              IDL do OVSDB protocol processing.
+
+          Transaction.UNCHANGED:
+
+              The transaction is complete.  (It didn't actually change the
+              database, so the IDL didn't send any request to the database
+              server.)
+
+          Transaction.ABORTED:
+
+              The caller previously called Transaction.abort().
+
+          Transaction.SUCCESS:
+
+              The transaction was successful.  The update made by the
+              transaction (and possibly other changes made by other database
+              clients) should already be visible in the IDL.
+
+          Transaction.TRY_AGAIN:
+
+              The transaction failed for some transient reason, e.g. because a
+              "verify" operation reported an inconsistency or due to a network
+              problem.  The caller should wait for a change to the database,
+              then compose a new transaction, and commit the new transaction.
+
+              Use Idl.change_seqno to wait for a change in the database.  It is
+              important to use its value *before* the initial call to
+              Transaction.commit() as the baseline for this purpose, because
+              the change that one should wait for can happen after the initial
+              call but before the call that returns Transaction.TRY_AGAIN, and
+              using some other baseline value in that situation could cause an
+              indefinite wait if the database rarely changes.
+
+          Transaction.NOT_LOCKED:
+
+              The transaction failed because the IDL has been configured to
+              require a database lock (with Idl.set_lock()) but didn't
+              get it yet or has already lost it.
 
         Committing a transaction rolls back all of the changes that it made to
 
         Committing a transaction rolls back all of the changes that it made to
-        the Idl's copy of the database.  If the transaction commits
+        the IDL's copy of the database.  If the transaction commits
         successfully, then the database server will send an update and, thus,
         successfully, then the database server will send an update and, thus,
-        the Idl will be updated with the committed changes."""
+        the IDL will be updated with the committed changes."""
         # The status can only change if we're the active transaction.
         # (Otherwise, our status will change only in Idl.run().)
         if self != self.idl.txn:
         # The status can only change if we're the active transaction.
         # (Otherwise, our status will change only in Idl.run().)
         if self != self.idl.txn:
@@ -849,6 +940,12 @@ class Transaction(object):
         return self._status
 
     def commit_block(self):
         return self._status
 
     def commit_block(self):
+        """Attempts to commit this transaction, blocking until the commit
+        either succeeds or fails.  Returns the final commit status, which may
+        be any Transaction.* value other than Transaction.INCOMPLETE.
+
+        This function calls Idl.run() on this transaction'ss IDL, so it may
+        cause Idl.change_seqno to change."""
         while True:
             status = self.commit()
             if status != Transaction.INCOMPLETE:
         while True:
             status = self.commit()
             if status != Transaction.INCOMPLETE:
@@ -862,6 +959,9 @@ class Transaction(object):
             poller.block()
 
     def get_increment_new_value(self):
             poller.block()
 
     def get_increment_new_value(self):
+        """Returns the final (incremented) value of the column in this
+        transaction that was set to be incremented by Row.increment.  This
+        transaction must have committed successfully."""
         assert self._status == Transaction.SUCCESS
         return self._inc_new_value
 
         assert self._status == Transaction.SUCCESS
         return self._inc_new_value