CDRIVER-5993 Support client metadata appends post-initialization#2261
Open
eramongodb wants to merge 7 commits intomongodb:masterfrom
Open
CDRIVER-5993 Support client metadata appends post-initialization#2261eramongodb wants to merge 7 commits intomongodb:masterfrom
eramongodb wants to merge 7 commits intomongodb:masterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves CDRIVER-5993 and all associated tickets (CDRIVER-4142, CDRIVER-6089, CDRIVER-6186, and CDRIVER-6277).
Adds new public API (
mongoc_client_append_metadata()andmongoc_client_pool_append_metadata()) to support appending client metadata to the handshake command after initialization of the client/pool object. The only metadata fields which may be appended-to are "client.driver.name", "client.driver.version", and "client.platform". This is consistent with the existing (global) metadata append API (mongoc_handshake_data_append()).Note
Although unified spec tests have been synced, they provide little value due to the way they are currently designed + the lack of CMAP event support.
The mongoc library currently assumes all client metadata used in handshakes remain unchanged after global initialization (the metadata is "frozen") with
_mongoc_handshake_get()(gMongocHandshake), with the sole exception of the appname. The appname is set at most once using an atomic compare-and-swap (CAS) where the expected value is alwaysNULL: any not-first attempts to set the value will fail due to the pointer not being null.This PR proposes using a similar compare-and-swap pattern for the handshake command owned by
mongoc_topology_scanner_t::handshake_cmd, but roughly in the form of a "double-checked lock" (the checks guard modification ofhandshake_cmd, not the locking ofhandshake_cmd_mut) by reusing the existing mutexmongoc_topology_scanner_t::handshake_mut(rather than by attempting to refactormongoc_topology_scanner_tinto using an atomic pointer). This results in a mutex-based rough-equivalent to the atomic pointer CAS. This pattern is in fact already being used by_mongoc_topology_scanner_dup_handshake_cmd(), with the caveat that its "failure" condition is to do nothing, as it assumes the resulting handshake command is identical (always initialized from the same frozen_mongoc_handshake_get()). The new API reuses this behavior (now extracted into the common_initialize_handshake_cmd()helper function for the first initialization ofhandshake_cmd) for the append operation. However, the append operation is retried with the updated metadata when the second check fails, resulting in sequential consistency. For clarity, all critical code which implements the double-checked lock behavior are documented with// Double-checked lock: ...comments.To facilitate reuse of existing metadata append behavior, internal calls to
_mongoc_handshake_get()are extracted out of helper function implementations into parameters, up to the critical function_build_handshake_cmd()(used by_mongoc_topology_scanner_dup_handshake_cmd()), which is responsible for building the BSON document assigned tomongoc_topology_scanner_t::handshake_cmd. This PR proposes reusing the_build_handshake_cmd()for the new metadata append API implemented by_mongoc_topology_scanner_append_metadata()rather than refactoring code currently applied specifically togMongocHandshake.To avoid breaking changes to existing (global) metadata append behavior, the new API uses a completely independent code path which operates on individual (copies of) fields of
mongoc_topology_scanner_t::handshake_cmd, not ongMongocHandshake.gMongocHandshakeremains unchanged after it is frozen. However, although code pertaining togMongocHandshakeis mostly unchanged, the_append_platform_field()function did require changes in order to correctly delimit appendable metadata from thecopmiler_infoandflagsmetadata that mongoc unconditionally attempts to append afterplatform(when they fit). This is a necessary breaking change in order to support correct platform string delimiting behavior per spec: see the changes to thetest_mongoc_platform_truncate()test for how this affects platform string behavior (note: there is still no delimiter betweencompiler_infoandflags).Prose tests are implemented under
"/MongoDB/handshake/metadata_append"intest-mongoc-handshake.c. The prose test spec usesmaxIdleTimeMSto trigger new handshakes; mongoc does not support this behavior, so instead the prose tests uses mock server hangups to force reconnections. When the prose tests specify initializing a client with metadata,mongoc_handshake_data_append()is used; when the prose tests specify appending metadata to an already-initialized client,mongoc_client_metadata_append()is used.As a drive-by fix/improvement,
test_mongoc_handshake_data_append_null_args()("/MongoDB/handshake/null_args") is updated to use_override_host_platform_os()instead of_reset_handshake()so that the behavior is more reliable across platforms where OS-specific varying field lengths can cause unexpected handshake truncation leading to test failure.