Disable allowUnconfirmed where we don’t want to support it (yet)

We want to enable `allowUnconfirmed` for puts and deletes in implicit
transactions.

This means disabling `allowUnconfirmed` for:
* anything in an explicit transaction.  The API for explicit
  transactions inherits the `allowUnconfirmed` option, so theoretically
  we could make all of the writes in a transaction be unconfirmed, and
  therefore the commit would be unconfirmed.  We may want to support
  this in the future
* `setAlarm`.  We may want to support this in the future.
* `deleteAll`.  This is probably difficult to support.

There is an edge case where multi-put creates its own explicit
transaction that will disable `allowUnconfirmed`.  This should
probably be fixed to use an implicit transaction instead.

When we override the `allowUnconfirmed` option to disable it, we log a
message.  I plan to look at these messages to see if they’re ever
actually logged.

Author: justin-mp

src/workerd/io/actor-sqlite.c++

@@ -25,6 +25,14 @@ static bool willFireEarlier(kj::Maybe<kj::Date> alarm1, kj::Maybe<kj::Date> alar
   return alarm1.orDefault(kj::maxValue) < alarm2.orDefault(kj::maxValue);
 }
 
+// Set options.allowUnconfirmed to false and log a reason why.
+void disableAllowUnconfirmed(ActorCacheOps::WriteOptions& options, kj::StringPtr reason) {
+  if (options.allowUnconfirmed) {
+    KJ_LOG(WARNING, "NOSENTRY allowUnconfirmed disabled", reason);
+    options.allowUnconfirmed = false;
+  }
+}
+
 }  // namespace
 
 ActorSqlite::ActorSqlite(kj::Own<SqliteDatabase> dbParam,
@@ -471,6 +479,9 @@ kj::OneOf<ActorCacheOps::GetResultList, kj::Promise<ActorCacheOps::GetResultList
 
 kj::Maybe<kj::Promise<void>> ActorSqlite::put(Key key, Value value, WriteOptions options) {
   requireNotBroken();
+  if (currentTxn.is<ExplicitTxn*>()) {
+    disableAllowUnconfirmed(options, "single put is using an already-existing ExplicitTxn");
+  }
   kv.put(key, value, {.allowUnconfirmed = options.allowUnconfirmed});
   return kj::none;
 }
@@ -482,10 +493,19 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::put(kj::Array<KeyValuePair> pairs, Wri
     if (currentTxn.is<NoTxn>()) {
       // If we are not in a transcation, let's use an ExplicitTxn, which should rollback automatically
       // if some put fails.
+
+      // TODO(someday) this should really start an ImplicitTxn, which has the advantage of
+      // supporting allowUnconfirmed.
+      disableAllowUnconfirmed(options, "multi put will create an ExplicitTxn");
+
       auto txn = startTransaction();
       txn->put(kj::mv(pairs), options);
       txn->commit();
     } else {
+      if (currentTxn.is<ExplicitTxn*>()) {
+        disableAllowUnconfirmed(options, "multi put is using an already-existing ExplicitTxn");
+      }
+
       // If we are in a transaction, let's just set a SAVEPOINT that we can rollback to if needed.
       db->run(
           {.regulator = SqliteDatabase::TRUSTED}, kj::str("SAVEPOINT _cf_put_multiple_savepoint"));
@@ -516,12 +536,20 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::put(kj::Array<KeyValuePair> pairs, Wri
 kj::OneOf<bool, kj::Promise<bool>> ActorSqlite::delete_(Key key, WriteOptions options) {
   requireNotBroken();
 
+  if (currentTxn.is<ExplicitTxn*>()) {
+    disableAllowUnconfirmed(options, "single delete is using an already-existing ExplicitTxn");
+  }
+
   return kv.delete_(key, {.allowUnconfirmed = options.allowUnconfirmed});
 }
 
 kj::OneOf<uint, kj::Promise<uint>> ActorSqlite::delete_(kj::Array<Key> keys, WriteOptions options) {
   requireNotBroken();
 
+  if (currentTxn.is<ExplicitTxn*>()) {
+    disableAllowUnconfirmed(options, "multi delete put is using an already-existing ExplicitTxn");
+  }
+
   uint count = 0;
   for (auto& key: keys) {
     count += kv.delete_(key, {.allowUnconfirmed = options.allowUnconfirmed});
@@ -533,6 +561,8 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::setAlarm(
     kj::Maybe<kj::Date> newAlarmTime, WriteOptions options) {
   requireNotBroken();
 
+  disableAllowUnconfirmed(options, "setAlarm is not supported");
+
   // TODO(someday): When deleting alarm data in an otherwise empty database, clear the database to
   // free up resources?
 
@@ -556,6 +586,8 @@ kj::Own<ActorCacheInterface::Transaction> ActorSqlite::startTransaction() {
 ActorCacheInterface::DeleteAllResults ActorSqlite::deleteAll(WriteOptions options) {
   requireNotBroken();
 
+  disableAllowUnconfirmed(options, "deleteAll is not supported");
+
   // kv.deleteAll() clears the database, so we need to save and possibly restore alarm state in
   // the metadata table, to try to match the behavior of ActorCache, which preserves the set alarm
   // when running deleteAll().
@@ -827,22 +859,27 @@ kj::OneOf<ActorCacheOps::GetResultList, kj::Promise<ActorCacheOps::GetResultList
 }
 kj::Maybe<kj::Promise<void>> ActorSqlite::ExplicitTxn::put(
     Key key, Value value, WriteOptions options) {
+  disableAllowUnconfirmed(options, "single put in ExplicitTxn not supported");
   return actorSqlite.put(kj::mv(key), kj::mv(value), options);
 }
 kj::Maybe<kj::Promise<void>> ActorSqlite::ExplicitTxn::put(
     kj::Array<KeyValuePair> pairs, WriteOptions options) {
+  disableAllowUnconfirmed(options, "multi put in ExplicitTxn not supported");
   return actorSqlite.put(kj::mv(pairs), options);
 }
 kj::OneOf<bool, kj::Promise<bool>> ActorSqlite::ExplicitTxn::delete_(
     Key key, WriteOptions options) {
+  disableAllowUnconfirmed(options, "single delete in ExplicitTxn not supported");
   return actorSqlite.delete_(kj::mv(key), options);
 }
 kj::OneOf<uint, kj::Promise<uint>> ActorSqlite::ExplicitTxn::delete_(
     kj::Array<Key> keys, WriteOptions options) {
+  disableAllowUnconfirmed(options, "multi delete in ExplicitTxn not supported");
   return actorSqlite.delete_(kj::mv(keys), options);
 }
 kj::Maybe<kj::Promise<void>> ActorSqlite::ExplicitTxn::setAlarm(
     kj::Maybe<kj::Date> newAlarmTime, WriteOptions options) {
+  disableAllowUnconfirmed(options, "setAlarm in ExplicitTxn not supported");
   return actorSqlite.setAlarm(newAlarmTime, options);
 }