CDRIVER-4110: Default for causal_consistency depends on snapshot (#838)

* Don't force casualConsistency=false when creating session with snapshot=true

This was originally added in d5f73e1

* Relocate session snapshot prose test

This prose test is defined in the sessions spec, not transactions. It also does not require the server to support transactions.

* Default for causal_consistency depends on snapshot

Use mongoc_optional_t for causal_consistency and snapshot session opts to track whether or not values have been set.

This also removes explicitly setting causal_consistency to true in _mongoc_client_session_new, since mongoc_session_opts_get_causal_consistency now enforces the default value.

Author: jmikola

src/libmongoc/src/mongoc/mongoc-client-session-private.h

@@ -38,14 +38,9 @@ struct _mongoc_transaction_opt_t {
    int64_t max_commit_time_ms;
 };
 
-typedef enum {
-   MONGOC_SESSION_NO_OPTS = 0,
-   MONGOC_SESSION_CAUSAL_CONSISTENCY = (1 << 0),
-   MONGOC_SESSION_SNAPSHOT = (2 << 0),
-} mongoc_session_flag_t;
-
 struct _mongoc_session_opt_t {
-   mongoc_session_flag_t flags;
+   mongoc_optional_t causal_consistency;
+   mongoc_optional_t snapshot;
    mongoc_transaction_opt_t default_txn_opts;
 };
 

src/libmongoc/src/mongoc/mongoc-client-session.c

@@ -403,7 +403,14 @@ mongoc_session_opts_get_causal_consistency (const mongoc_session_opt_t *opts)
 
    BSON_ASSERT (opts);
 
-   RETURN (!!(opts->flags & MONGOC_SESSION_CAUSAL_CONSISTENCY));
+   /* Causal Consistency spec: If no value is provided for causalConsistency
+    * and snapshot reads are not requested a value of true is implied. */
+   if (!mongoc_optional_is_set (&opts->causal_consistency) &&
+       !mongoc_optional_value (&opts->snapshot)) {
+      RETURN (true);
+   }
+
+   RETURN (mongoc_optional_value (&opts->causal_consistency));
 }
 
 bool
@@ -413,7 +420,7 @@ mongoc_session_opts_get_snapshot (const mongoc_session_opt_t *opts)
 
    BSON_ASSERT (opts);
 
-   RETURN (!!(opts->flags & MONGOC_SESSION_SNAPSHOT));
+   RETURN (mongoc_optional_value (&opts->snapshot));
 }
 
 void
@@ -424,11 +431,7 @@ mongoc_session_opts_set_causal_consistency (mongoc_session_opt_t *opts,
 
    BSON_ASSERT (opts);
 
-   if (causal_consistency) {
-      opts->flags |= MONGOC_SESSION_CAUSAL_CONSISTENCY;
-   } else {
-      opts->flags &= ~MONGOC_SESSION_CAUSAL_CONSISTENCY;
-   }
+   mongoc_optional_set_value (&opts->causal_consistency, causal_consistency);
 
    EXIT;
 }
@@ -440,11 +443,7 @@ mongoc_session_opts_set_snapshot (mongoc_session_opt_t *opts, bool snapshot)
 
    BSON_ASSERT (opts);
 
-   if (snapshot) {
-      opts->flags |= MONGOC_SESSION_SNAPSHOT;
-   } else {
-      opts->flags &= ~MONGOC_SESSION_SNAPSHOT;
-   }
+   mongoc_optional_set_value (&opts->snapshot, snapshot);
 
    EXIT;
 }
@@ -454,11 +453,8 @@ mongoc_session_opts_new (void)
 {
    mongoc_session_opt_t *opts = bson_malloc0 (sizeof (mongoc_session_opt_t));
 
-   /* Driver Sessions Spec: causal consistency is true by default */
-   mongoc_session_opts_set_causal_consistency (opts, true);
-
-   /* Snapshot Reads Spec: snapshot is false by default */
-   mongoc_session_opts_set_snapshot (opts, false);
+   mongoc_optional_init (&opts->causal_consistency);
+   mongoc_optional_init (&opts->snapshot);
 
    return opts;
 }
@@ -513,7 +509,8 @@ static void
 _mongoc_session_opts_copy (const mongoc_session_opt_t *src,
                            mongoc_session_opt_t *dst)
 {
-   dst->flags = src->flags;
+   mongoc_optional_copy (&src->causal_consistency, &dst->causal_consistency);
+   mongoc_optional_copy (&src->snapshot, &dst->snapshot);
    txn_opts_copy (&src->default_txn_opts, &dst->default_txn_opts);
 }
 
@@ -800,22 +797,22 @@ _mongoc_client_session_new (mongoc_client_t *client,
    session->client_session_id = client_session_id;
    bson_init (&session->cluster_time);
 
+   mongoc_optional_init (&session->opts.causal_consistency);
+   mongoc_optional_init (&session->opts.snapshot);
    txn_opts_set (&session->opts.default_txn_opts,
                  client->read_concern,
                  client->write_concern,
                  client->read_prefs,
                  DEFAULT_MAX_COMMIT_TIME_MS);
 
    if (opts) {
-      session->opts.flags = opts->flags;
+      mongoc_optional_copy (&opts->causal_consistency, &session->opts.causal_consistency);
+      mongoc_optional_copy (&opts->snapshot, &session->opts.snapshot);
       txn_opts_set (&session->opts.default_txn_opts,
                     opts->default_txn_opts.read_concern,
                     opts->default_txn_opts.write_concern,
                     opts->default_txn_opts.read_prefs,
                     opts->default_txn_opts.max_commit_time_ms);
-   } else {
-      /* sessions are causally consistent by default */
-      session->opts.flags = MONGOC_SESSION_CAUSAL_CONSISTENCY;
    }
 
    /* snapshot_time_set is false by default */

src/libmongoc/tests/test-mongoc-client-session.c

@@ -34,7 +34,7 @@ test_session_opts_clone (void)
 
    opts = mongoc_session_opts_new ();
    clone = mongoc_session_opts_clone (opts);
-   /* causal is enabled by default */
+   /* causalConsistency is enabled by default if snapshot is not enabled */
    BSON_ASSERT (mongoc_session_opts_get_causal_consistency (clone));
    mongoc_session_opts_destroy (clone);
 
@@ -47,6 +47,31 @@ test_session_opts_clone (void)
 }
 
 
+static void
+test_session_opts_causal_consistency_and_snapshot (void)
+{
+   mongoc_session_opt_t *opts;
+
+   opts = mongoc_session_opts_new ();
+   /* causalConsistency is enabled by default if snapshot is not enabled */
+   BSON_ASSERT (mongoc_session_opts_get_causal_consistency (opts));
+   BSON_ASSERT (!mongoc_session_opts_get_snapshot (opts));
+
+   /* causalConsistency is disabled by default if snapshot is enabled */
+   mongoc_session_opts_set_snapshot (opts, true);
+   BSON_ASSERT (!mongoc_session_opts_get_causal_consistency (opts));
+   BSON_ASSERT (mongoc_session_opts_get_snapshot (opts));
+
+   /* causalConsistency and snapshot can both be enabled, although this will
+    * result in an error when starting the session. */
+   mongoc_session_opts_set_causal_consistency (opts, true);
+   BSON_ASSERT (mongoc_session_opts_get_causal_consistency (opts));
+   BSON_ASSERT (mongoc_session_opts_get_snapshot (opts));
+
+   mongoc_session_opts_destroy (opts);
+}
+
+
 static void
 test_session_no_crypto (void *ctx)
 {
@@ -2707,12 +2732,44 @@ test_session_dirty (void *unused)
    _test_session_dirty_helper (false /* retry succceeds */);
 }
 
+void
+test_sessions_snapshot_prose_test_1 (void *ctx)
+{
+   mongoc_client_t *client = NULL;
+   mongoc_session_opt_t *session_opts = NULL;
+   bson_error_t error;
+   bool r;
+
+   client = test_framework_new_default_client ();
+   BSON_ASSERT (client);
+
+   session_opts = mongoc_session_opts_new ();
+   mongoc_session_opts_set_causal_consistency (session_opts, true);
+   mongoc_session_opts_set_snapshot (session_opts, true);
+
+   /* assert that starting session with causal consistency and snapshot enabled
+    * results in an error. */
+   r = mongoc_client_start_session (client, session_opts, &error);
+   ASSERT (!r);
+   ASSERT_ERROR_CONTAINS (
+      error,
+      MONGOC_ERROR_CLIENT,
+      MONGOC_ERROR_CLIENT_SESSION_FAILURE,
+      "Only one of causal consistency and snapshot can be enabled.");
+
+   mongoc_session_opts_destroy (session_opts);
+   mongoc_client_destroy (client);
+}
+
 void
 test_session_install (TestSuite *suite)
 {
    char resolved[PATH_MAX];
 
    TestSuite_Add (suite, "/Session/opts/clone", test_session_opts_clone);
+   TestSuite_Add (suite,
+                  "/Session/opts/causal_consistency_and_snapshot",
+                  test_session_opts_causal_consistency_and_snapshot);
    TestSuite_AddFull (suite,
                       "/Session/no_crypto",
                       test_session_no_crypto,
@@ -3079,4 +3136,12 @@ test_session_install (TestSuite *suite)
       test_framework_skip_if_no_failpoint,
       /* Tests with retryable writes, requires non-standalone. */
       test_framework_skip_if_single);
+
+   TestSuite_AddFull (suite,
+                      "/Session/snapshot/prose_test_1",
+                      test_sessions_snapshot_prose_test_1,
+                      NULL,
+                      NULL,
+                      test_framework_skip_if_no_sessions,
+                      test_framework_skip_if_no_crypto);
 }

src/libmongoc/tests/test-mongoc-transactions.c

@@ -1166,35 +1166,6 @@ test_max_commit_time_ms_is_reset (void *ctx)
    mock_rs_destroy (rs);
 }
 
-void
-test_snapshot_session_prose_1 (void *ctx)
-{
-   mongoc_client_t *client = NULL;
-   mongoc_session_opt_t *session_opts = NULL;
-   bson_error_t error;
-   bool r;
-
-   client = test_framework_new_default_client ();
-   BSON_ASSERT (client);
-
-   session_opts = mongoc_session_opts_new ();
-   mongoc_session_opts_set_causal_consistency (session_opts, true);
-   mongoc_session_opts_set_snapshot (session_opts, true);
-
-   /* assert that starting session with causal consistency and snapshot enabled
-    * results in an error. */
-   r = mongoc_client_start_session (client, session_opts, &error);
-   ASSERT (!r);
-   ASSERT_ERROR_CONTAINS (
-      error,
-      MONGOC_ERROR_CLIENT,
-      MONGOC_ERROR_CLIENT_SESSION_FAILURE,
-      "Only one of causal consistency and snapshot can be enabled.");
-
-   mongoc_session_opts_destroy (session_opts);
-   mongoc_client_destroy (client);
-}
-
 void
 test_transactions_install (TestSuite *suite)
 {
@@ -1286,10 +1257,4 @@ test_transactions_install (TestSuite *suite)
                       NULL,
                       NULL,
                       test_framework_skip_if_no_crypto);
-   TestSuite_AddFull (suite,
-                      "/transactions/snapshot_sessions_prose_1",
-                      test_snapshot_session_prose_1,
-                      NULL,
-                      NULL,
-                      test_framework_skip_if_no_txns);
 }

src/libmongoc/tests/unified/entity-map.c

@@ -597,8 +597,6 @@ session_opts_new (bson_t *bson, bson_error_t *error)
       mongoc_session_opts_set_causal_consistency (opts, *causal_consistency);
    }
    if (snapshot) {
-      /* Set causal consistency to false to allow snapshot */
-      mongoc_session_opts_set_causal_consistency (opts, false);
       mongoc_session_opts_set_snapshot (opts, *snapshot);
    }