Bug 1687001 Comment 4 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`.  I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following:
# duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork`
# add a mutex protected the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync.

Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable?

Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient.

Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic updates at least. This is problematic as well, but this is not used in regular release builds.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`.  I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following:
1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork`
2. add a mutex protected the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync.

Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable?

Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient.

Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic updates at least. This is problematic as well, but this is not used in regular release builds.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`.  I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following:
1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork`
2. add a mutex protected the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync.

Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable?

Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient.

Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic accesses at least. This is problematic as well, but this is not used in regular release builds.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`.  I think it's hard to avoid that we call `FullDatabaseMetadata::Duplicate` on the background thread. We could do either of the following:
1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork`
2. add a mutex protecting the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync.

Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable?

Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient.

Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic accesses at least. This is problematic as well, but this is not used in regular release builds.
So, `FullDatabaseMetadata::Duplicate` must necessarily read `mCommittedAutoIncrementId`/`mNextAutoIncrementId`.  I think it's hard to avoid that we call `OpenDatabaseOp::BeginVersionChange` on the background thread. We could do either of the following:
1. duplicate the metadata already in `OpenDatabaseOp::DoDatabaseWork`
2. add a mutex protecting the two members, so they can safely be accessed from any thread. Just making each of them `Atomic` wouldn't suffice, since they need to be kept in sync.

Jan, what do you think? Is there a problem with either of these options? If not, which do you think is preferable?

Moving `CommitOrRollbackAutoIncrementCounts` to the background thread instead would require another two events to be introduced for proper serialization, which seems to be far more complicated and inefficient.

Note that `OpenDatabaseOp::AssertMetadataConsistency` contains assertions reading `mCommittedAutoIncrementId`/`mNextAutoIncrementId` on the background thread as well. It even contains a comment on this, but seems to assume atomic accesses at least. This is problematic as well, but this is not used in regular release builds.

Back to Bug 1687001 Comment 4