[tahoe-lafs-trac-stream] [tahoe-lafs] #1864: turn off the AUTOINCREMENT feature in our use of sqlite?

[tahoe-lafs]

#1864: turn off the AUTOINCREMENT feature in our use of sqlite?
-------------------------+-------------------------------------------------
     Reporter:  zooko    |      Owner:  warner
         Type:           |     Status:  new
  enhancement            |  Milestone:  1.11.0
     Priority:  normal   |    Version:  1.9.2
    Component:  code-    |   Keywords:  sqlite design-review-needed leasedb
  storage                |
   Resolution:           |
Launchpad Bug:           |
-------------------------+-------------------------------------------------

Old description:

> I just discovered this doc:
>
> http://sqlite.org/autoinc.html
>
> There are two different ways that sqlite can provide automatic "ROWID".
> The standard one is a tad more efficient for sqlite to implement, the
> other one — AUTOINCREMENT — makes sure that you don't get the same ROWID
> twice in a row if you create a new row, then delete it, then create
> another new row. (The AUTOINCREMENT one also has a different behavior if
> you manually set your ROWID and you set it to the largest integer that
> sqlite can handle.)
>
> As far as I can think, we don't mind if the same ROWID is used to refer
> both to a row that currently exists and a row that used to exist but does
> no longer. (Because we never delete a row without also deleting or
> updating all other references to it. Right?)
>
> If I'm right, which I'm not sure of, then we could stop specifying to
> sqlite that we need the AUTOINCREMENT style, and instead use the standard
> and slightly more efficient style of ROWID.

New description:

 I just discovered this doc:

 http://sqlite.org/autoinc.html

 There are two different ways that sqlite can provide automatic "ROWID".
 The standard one is a tad more efficient for sqlite to implement, the
 other one — AUTOINCREMENT — makes sure that you don't get the same ROWID
 twice in a row if you create a new row, then delete it, then create
 another new row. (The AUTOINCREMENT one also has a different behavior if
 you manually set your ROWID and you set it to the largest integer that
 sqlite can handle.)

 As far as I can think, we don't mind if the same ROWID is used to refer
 both to a row that currently exists and a row that used to exist but does
 no longer. (Because we never delete a row without also deleting or
 updating all other references to it. Right?)

 If I'm right, which I'm not sure of, then we could stop specifying to
 sqlite that we need the AUTOINCREMENT style, and instead use the standard
 and slightly more efficient style of ROWID.

--

Comment (by zooko):

 Replying to [comment:8 davidsarah]:
 > Note that in [https://github.com/davidsarah/tahoe-
 lafs/commit/b7cea9a3f86c5cc4cfd3910d0300a1008b7b5a13], I got rid of the
 AUTOINCREMENT key on the `leases` table, replacing it with a semantic
 primary key. So there are no longer any AUTOINCREMENT keys on leasedb
 tables where records are created at all frequently. Therefore, I think
 there is no available performance improvement from this change, at least
 for leasedb.
 >
 > If you (zooko or warner) agree, I'll just close this ticket as invalid.

 It seems to me that we should make this change:

 {{{
 zooko at spark ~/playground/tahoe-lafs $ git diff
 diff --git a/docs/backupdb.rst b/docs/backupdb.rst
 index 5a36b51..e47ca3b 100644
 --- a/docs/backupdb.rst
 +++ b/docs/backupdb.rst
 @@ -61,7 +61,7 @@ The database contains the following tables::

    CREATE TABLE caps
    (
 -   fileid integer PRIMARY KEY AUTOINCREMENT,
 +   fileid integer PRIMARY KEY,
     filecap varchar(256) UNIQUE    -- URI:CHK:...
    );

 diff --git a/src/allmydata/scripts/backupdb.py
 b/src/allmydata/scripts/backupdb.py
 index 75ee0d9..37180f9 100644
 --- a/src/allmydata/scripts/backupdb.py
 +++ b/src/allmydata/scripts/backupdb.py
 @@ -27,7 +27,7 @@ CREATE TABLE local_files -- added in v1

  CREATE TABLE caps -- added in v1
  (
 - fileid INTEGER PRIMARY KEY AUTOINCREMENT,
 + fileid INTEGER PRIMARY KEY,
   filecap VARCHAR(256) UNIQUE       -- URI:CHK:...
  );

 }}}

 Not primarily for efficiency (since creation of new caps during a backup,
 or a future "sync" or a future "cp -r --with-db" is not high frequency),
 but primarily for simplicity. We don't need the {{{AUTOINCREMENT}}}
 feature of sqlite, so we shouldn't say that we want it (and it might slow
 a backupdb file-creation operation down by a few hundred nanoseconds
 occasionally).

 Daira, Brian: what do you say?

-- 
Ticket URL: <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1864#comment:9>
tahoe-lafs <https://tahoe-lafs.org>
secure decentralized storage