> > Yes, INSERT OR IGNORE supported by all versions. I plan to start a separate
> > thread about this topic.
> >
>
> As mentioned above, I think we can optimize the insert statement for the
> case of a constraint violation. Now we use the 'FAIL' conflict
> resolution algorithm [1] and explicitly handle
> SVN_ERR_SQLITE_CONSTRAINT error in svn_fs_fs__set_rep_reference().
> Because of this, we have an extra svn_fs_fs__get_rep_reference() call.
We do have an extra call when a duplicate rep is attempted to be
inserted into the rep-cache. When does that happen? I know it will
happen when called from build-repcache; what other use-cases trigger the
codepath you changed? Committing two files with equal contents
sequentially doesn't call svn_fs_fs__set_rep_reference() at all for the
second file.
> As far as I know, svn_sqlite__step() in svn_sqlite__insert() is the
> only place where the error can occur. Based on this, I suggest to use
> the 'IGNORE' conflict resolution algorithm [1] and remove the
> SVN_ERR_SQLITE_CONSTRAINT handling, because the error should not occur
> now. It seems to me the change is quite safe, because the behavior is
> well covered by tests.
>
What tests is this behaviour covered by?
> rep-cache.db insert optimization.
>
> Use the 'IGNORE' conflict resolution algorithm [1] for STMT_SET_REP
> and remove SVN_ERR_SQLITE_CONSTRAINT handling, because the error
> should not occur now. It brings a slight performance improvement,
What scenario does this bring a performance improvement in? What is the
improvement, quantitatively?
> because we remove an extra svn_fs_fs__get_rep_reference() call.
>
> [1] https://www.sqlite.org/lang_conflict.html
>
> * subversion/libsvn_fs_fs/rep-cache-db.sql
> (STMT_SET_REP): Use the 'IGNORE' conflict resolution algorithm.
>
> * subversion/libsvn_fs_fs/rep-cache.c
> (svn_fs_fs__set_rep_reference): Remove SVN_ERR_SQLITE_CONSTRAINT handling.
Cheers,
Daniel
Received on 2020-03-26 21:27:34 CET