[CDRIVER-1337] Add configure flag for unstable MongoDB 3.4 support Created: 17/Jun/16  Updated: 08/Aug/16  Resolved: 08/Aug/16

Status: Closed
Project: C Driver
Component/s: Build
Affects Version/s: 1.4.0
Fix Version/s: 1.4.0

Type: New Feature Priority: Major - P3
Reporter: A. Jesse Jiryu Davis Assignee: A. Jesse Jiryu Davis
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to CXX-969 Fix Appveyor for C++ drivers for libm... Closed
is related to CXX-873 Build against the master branch of th... Closed
is related to CDRIVER-1393 Move MongoDB Handshake data behind ex... Closed
is related to CDRIVER-1410 Add evergreen to build matrix compile... Closed

 Description   

Add a configure flag to libbson and libmongoc's Autotools and CMake build systems, something like --enable-experimental-features that is off by default, undocumented, and enables decimal128 and other MongoDB 3.4 features in the C Driver on master. Unhide these features for the 1.5 release.



 Comments   
Comment by A. Jesse Jiryu Davis [ 08/Aug/16 ]

Closing unless anyone objects to the current solution.

Comment by Hannes Magnusson [ 05/Aug/16 ]

I'm fine with hiding it from our configure flags. The original idea was to have it totally hidden and require manually adding it to your CFLAGS before running ./configure. That idea proved difficult because we have to export different symbols, along with various tedious details.

I can't think of a way to remove it from the ./configure --help listing, while keeping it as a supported flag. Is that possible?

Trying to extract an actionable item from your comment, best I can suggest is to add extra experimental disclaimer when using the flag?
It already says its experimental, but we can throw in some caps and stronger language?

Since humans don't edit the mongoc-config.h, I'm not sure what difference it makes if its called __MONGOC_INTERNAL_EXPERIMENT or MONGOC_EXPERIMENTAL. If you think its worth the change in mongoc, libbson, phpc, and the outstanding c++11 PR, then.. ok.

Comment by David Golden [ 05/Aug/16 ]

After some reflection and discussions with acm and jesse, and following the principle of "strong opinions, lightly held", I've come around to a different way of thinking:

  • The C++11 driver will be using a development branch for its version 3.1 (with MongodB 3.4 features). It won't merge to master until after libmongoc 1.5 ships. Therefore, it doesn't need to use any conditional macros of its own or from libmongoc.
  • The reason we need conditional macros in libmongoc is so that during development we can test the C++11 3.1 branch against a libmongoc with those symbols available.
  • Effectively, the libmongoc flag is just a developer tool to enable some symbols (in lieu of all 3.4 work in libmongoc being on a dev branch) so we can build and test before libmongoc 1.5 is stable. It does not need to be consumed by general downstream users.
  • If it's not going to be consumed by downstream users, then we shouldn't use feature flags as a mental model. It should be coarse-grained and called "experimental" or even something more arcane and off-putting. (e.g. __MONGOC_INTERNAL_EXPERIMENT) It should be documented with strong warnings so that downstream users don't depend on it.
Comment by Hannes Magnusson [ 04/Aug/16 ]

I don't think its a good idea.
C++11 r3.1.0 will then unintentionally conditionally have MongoDB 3.4 support.

I think it needs to be deliberate decision if C++11 r3.1.0 has MongoDB 3.4 support or not. Not "maybe" and "it depends".

No driver should ship production ready driver with experimental features enabled by default. But thats effectively what happens when the feature macro is kept around for 1.5, then the experimental code in C++11 r3.1.0 that was work-in-progress suddenly has hardcoded 3.4 support which isn't complete.

Comment by A. Jesse Jiryu Davis [ 04/Aug/16 ]

Like I said, MONGODB_34_EXPERIMENTAL "is off by default in 1.4.0, In 1.5.0 it's on and you can't turn it off." It won't be removed.

Comment by Hannes Magnusson [ 04/Aug/16 ]

I'm not sure what #1 solves? If we remove the MONGODB_34_EXPERIMENTAL their code still won't work, so we'd have to keep it around forever or remove it "in a very coordinated" way.

In related note: When we originally added this I was in favor of independent flags, and still am. But it doesn't resolve the issues pointed out (which I don't think are issues).

The experimental flag is meant to be removed. Drivers using it are meant to remove it. It is meant as a one off batch of things that dependent drivers can start playing with, breaking, and developing initial support. Dependent drivers CANNOT advertise availability of these features behind experimental. We have to get these features out to dependent drivers as early as we can to give them the chance to work with them with plenty of time before MongoDB 3.4. Releasing them last week of Q3 when the server ships is cruel.

Once time comes for us to ship our, and dependent drivers, MongoDB 3.4 compatibility releases, the flag will be removed and dependent drivers are expected to remove the check and then announce its availability.

I have no issue with the code churn. It is removal of a code block that declared it experimental and intentionally promoting it to be fully valued feature of the driver, or removing the feature completely as it didn't make the cut for MongoDB 3.4.

Still checking for MONGOC_34_FEATURES or MONGOC_EXPERIMENTAL_HANDSHAKE_DATA when the feature has been removed, or dramatically changed for MongoDB 3.6 so we'd have to change the API isn't going to help you much.

And if we unconditionally would set the MONGOC_34_FEATURES flag in 1.5, the features would accidentally be promoted to fully qualified features in the dependent drivers in versions they had no intentions of supporting 3.4.

Comment by David Golden [ 04/Aug/16 ]

Bikeshed for #1: How about calling it MONGODB_34_FEATURES instead of MONGODB_34_EXPERIMENTAL. I.e. coarse-grained features rather than "experimental". It's going to be weird to see "experimental" locked on after it's not experimental anymore and this way even after depending on 1.5.0, we can just leave the flag alone.

Comment by A. Jesse Jiryu Davis [ 04/Aug/16 ]

Hannes, would you be willing to do this?

Various people have suggested that instead of a general "experimental" flag, we should have specific feature flags. david.golden convinced me in a GitHub comment.

I have two proposals:

Proposal one. Rename the flag MONGODB_34_EXPERIMENTAL. It's off by default in 1.4.0, In 1.5.0 it's on and you can't turn it off. Downstream drivers can #ifdef MONGODB_34_EXPERIMENTAL for now. Once they depend on 1.5.0 they can remove their ifdefs.

We can continue to step forward like this in the future: MONGODB_36_EXPERIMENTAL and so on.

I like proposal one because it's easy for us to search-and-replace our whole codebase, and it's easy for downstream drivers to search-and-replace theirs, instead of doing finicky work for each feature.

Proposal two. Feature flags:

  • BSON_EXPERIMENTAL_DECIMAL128
  • MONGOC_EXPERIMENTAL_COMMANDS_WRITE_CONCERN
  • MONGOC_EXPERIMENTAL_HANDSHAKE_DATA
  • MONGOC_EXPERIMENTAL_MAX_STALENESS
Comment by Githook User [ 27/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 disable aggregate_with_write_concern
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/f9425f38e4acfb50c470a8605b655d83af5c0d7e

Comment by Githook User [ 20/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 fix decimal128 JSON test
Branch: master
https://github.com/mongodb/libbson/commit/7fcb9573290ab62f34d4755850f616141d1be231

Comment by Githook User [ 20/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 completely disable decimal by default
Branch: master
https://github.com/mongodb/libbson/commit/03284d18bd67f52a69d50c07c9b60f0c83adec19

Comment by A. Jesse Jiryu Davis [ 20/Jul/16 ]

Need to disable BSON_ITER_HOLDS_DECIMAL128, VISIT_DECIMAL128.

Comment by Githook User [ 18/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 disable additional maxStalenessMS features
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/ba6c17e32e4fe419eaccd1f76abfea7f6208d6e2

Comment by Githook User [ 18/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 disable maxStalenessMS by default
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/dba96fbdf9a0008eaa978f72650a6845adc4dab0

Comment by Githook User [ 14/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 disable experimental features by default
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/887ed7e05e6cd23991e55382a8a2a4e2c96b0676

Comment by Githook User [ 07/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 build libbson with experimental features
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/3fb2dc4feedb36ea2f5980ebf675c82603075995

Comment by Githook User [ 07/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 export all symbols in libbson-experimental.def

I'd thought specifying two symbol files with CMake's add_library would
merge their symbol tables. In fact the last file wins, so all symbols
must be included in single symbol file.
Branch: master
https://github.com/mongodb/libbson/commit/e1c1516a64a39cc0cb8b9c31b14099a3c926cfb0

Comment by Githook User [ 06/Jul/16 ]

Author:

{u'username': u'bjori', u'name': u'Hannes Magnusson', u'email': u'bjori@php.net'}

Message: CDRIVER-1337 Fix double declaration of BSON_EXPERIMENTAL_FEATURES
Branch: master
https://github.com/mongodb/libbson/commit/50f08b4ed66f52416945f9c49b3a7a24eece3f95

Comment by Githook User [ 06/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 define BSON_EXPERIMENTAL_FEATURES
Branch: master
https://github.com/mongodb/libbson/commit/93f788252969c1ba6505fb7f681a91234d09fa90

Comment by Githook User [ 06/Jul/16 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-1337 disable future features by default
Branch: master
https://github.com/mongodb/libbson/commit/3dbe26172bad65bf9b5bd9d01e822b7b947ccb6a

Generated at Wed Feb 07 21:12:12 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.