CDRIVER-1082 proper bulk op splitting at the margin

ajdavis · ajdavis · commit fb44d117ef5e · 2016-02-05T01:51:34.000-05:00
diff --git a/NEWS b/NEWS
@@ -1,3 +1,15 @@
+mongo-c-driver 1.3.3
+====================
+
+It is my pleasure to announce MongoDB C Driver 1.3.3. This fixes a bug where
+a slightly-oversized bulk write operation was not split into batches; instead,
+it was sent whole to the server, which rejected it.
+
+Peace,
+
+  A. Jesse Jiryu Davis
+
+
 mongo-c-driver 1.3.2
 ====================
 
diff --git a/src/mongoc/mongoc-write-command.c b/src/mongoc/mongoc-write-command.c
@@ -51,6 +51,7 @@ static bson_t gEmptyWriteConcern = BSON_INITIALIZER;
 /* indexed by MONGOC_WRITE_COMMAND_DELETE, INSERT, UPDATE */
 static const char *gCommandNames[] = { "delete", "insert", "update"};
 static const char *gCommandFields[] = { "deletes", "documents", "updates"};
+static const uint32_t gCommandFieldLens[] = { 7, 9, 7 };
 
 static int32_t
 _mongoc_write_result_merge_arrays (uint32_t               offset,
@@ -589,14 +590,13 @@ _mongoc_write_command_will_overflow (uint32_t len_so_far,
                                      int32_t  max_bson_size,
                                      int32_t  max_write_batch_size)
 {
-   /* max BSON object size + 16k - 2 bytes for ending NUL bytes.
+   /* max BSON object size + 16k bytes.
     * server guarantees there is enough room: SERVER-10643
     */
-   int32_t max_cmd_size = max_bson_size + 16382;
+   int32_t max_cmd_size = max_bson_size + 16384;
 
    BSON_ASSERT (max_bson_size);
 
-
    if (len_so_far + document_len > max_cmd_size) {
       return true;
    } else if (max_write_batch_size > 0 &&
@@ -799,6 +799,7 @@ _mongoc_write_command(mongoc_write_command_t       *command,
    int32_t max_bson_obj_size;
    int32_t max_write_batch_size;
    int32_t min_wire_version;
+   uint32_t overhead;
    uint32_t key_len;
 
    ENTRY;
@@ -856,18 +857,25 @@ _mongoc_write_command(mongoc_write_command_t       *command,
                         !!command->flags.bypass_document_validation);
    }
 
-   if (!_mongoc_write_command_will_overflow (0,
+   /* 1 byte to specify array type, 1 byte for field name's null terminator */
+   overhead = cmd.len + 2 + gCommandFieldLens[command->type];
+
+   if (!_mongoc_write_command_will_overflow (overhead,
                                              command->documents->len,
                                              command->n_documents,
                                              max_bson_obj_size,
                                              max_write_batch_size)) {
       /* copy the whole documents buffer as e.g. "updates": [...] */
-      BSON_APPEND_ARRAY (&cmd,
+      bson_append_array (&cmd,
                          gCommandFields[command->type],
+                         gCommandFieldLens[command->type],
                          command->documents);
       i = command->n_documents;
    } else {
-      bson_append_array_begin (&cmd, gCommandFields[command->type], -1, &ar);
+      bson_append_array_begin (&cmd,
+                               gCommandFields[command->type],
+                               gCommandFieldLens[command->type],
+                               &ar);
 
       do {
          if (!BSON_ITER_HOLDS_DOCUMENT (&iter)) {
@@ -877,8 +885,8 @@ _mongoc_write_command(mongoc_write_command_t       *command,
          bson_iter_document (&iter, &len, &data);
          key_len = (uint32_t) bson_uint32_to_string (i, &key, str, sizeof str);
 
-         if (_mongoc_write_command_will_overflow (ar.len,
-                                                  key_len + len + 2,
+         if (_mongoc_write_command_will_overflow (overhead,
+                                                  key_len + len + 2 + ar.len,
                                                   i,
                                                   max_bson_obj_size,
                                                   max_write_batch_size)) {
diff --git a/tests/test-bulk.c b/tests/test-bulk.c
@@ -876,8 +876,65 @@ test_replace_one (bool ordered)
 }
 
 
+/*
+ * check that we include command overhead in msg size when deciding to split,
+ * CDRIVER-1082
+ */
 static void
 test_upsert_large (void)
+{
+   mongoc_bulk_operation_t *bulk;
+   mongoc_collection_t *collection;
+   mongoc_client_t *client;
+   bool has_write_cmds;
+   bson_t *selector = tmp_bson ("{'_id': 'aaaaaaaaaa'}");
+   size_t sz = 8396691;  /* a little over 8 MB */
+   char *large_str = bson_malloc (sz);
+   bson_t update = BSON_INITIALIZER;
+   bson_t child;
+   bson_error_t error;
+   int i;
+   bson_t reply;
+
+   client = test_framework_client_new ();
+   has_write_cmds = server_has_write_commands (client);
+   collection = get_test_collection (client, "test_upsert_large");
+   bulk = mongoc_collection_create_bulk_operation (collection, true, NULL);
+
+   bson_append_document_begin (&update, "$set", 4, &child);
+   bson_append_utf8 (&child, "big", 3, large_str, (int) sz);
+   bson_append_document_end (&update, &child);
+
+   /* two 8MB+ docs could fit in 16MB + 16K, if not for command overhead,
+    * check the driver splits into two msgs */
+   for (i = 0; i < 2; i++) {
+      mongoc_bulk_operation_update (bulk, selector, &update, true);
+   }
+
+   ASSERT_OR_PRINT ((bool) mongoc_bulk_operation_execute (bulk, &reply, &error),
+                    error);
+
+   ASSERT_MATCH (&reply, "{'nInserted': 0,"
+                         " 'nMatched':  1,"
+                         " 'nRemoved':  0,"
+                         " 'nUpserted': 1,"
+                         " 'upserted': [{'index': 0, '_id': 'aaaaaaaaaa'}],"
+                         " 'writeErrors': []}");
+
+   check_n_modified (has_write_cmds, &reply, 0);
+   ASSERT_COUNT (1, collection);
+
+   bson_destroy (&reply);
+   mongoc_bulk_operation_destroy (bulk);
+   mongoc_collection_destroy (collection);
+   mongoc_client_destroy (client);
+   bson_destroy (&update);
+   bson_free (large_str);
+}
+
+
+static void
+test_upsert_huge (void)
 {
    mongoc_bulk_operation_t *bulk;
    mongoc_collection_t *collection;
@@ -897,7 +954,7 @@ test_upsert_large (void)
    assert (client);
    has_write_cmds = server_has_write_commands (client);
 
-   collection = get_test_collection (client, "test_upsert_large");
+   collection = get_test_collection (client, "test_upsert_huge");
    assert (collection);
 
    bulk = mongoc_collection_create_bulk_operation (collection, true, NULL);
@@ -2852,6 +2909,8 @@ test_bulk_install (TestSuite *suite)
                   test_upsert_unordered);
    TestSuite_Add (suite, "/BulkOperation/upsert_large",
                   test_upsert_large);
+   TestSuite_Add (suite, "/BulkOperation/upsert_huge",
+                  test_upsert_huge);
    TestSuite_Add (suite, "/BulkOperation/upserted_index_ordered",
                   test_upserted_index_ordered);
    TestSuite_Add (suite, "/BulkOperation/upserted_index_unordered",
