telefonicaid / fiware-orion

Context Broker and CEF building block for context data management, providing NGSI interfaces.
https://fiware-orion.rtfd.io/
GNU Affero General Public License v3.0
212 stars 265 forks source link

Why using older version of mongodb driver. #3132

Closed saurabhjangir closed 3 years ago

saurabhjangir commented 6 years ago

According to http://fiware-orion.readthedocs.io/en/latest/admin/build_source we are using mongo-cxx-driver-legacy-1.1.2 which is not supported by the community member now. This driver is replaced by C++11 driver. You can refer https://jira.mongodb.org/browse/CXX-1526 for this. Also, backward compatibility is not supported for MongoDB 3.4 and later releases. Because of Issue #3119 , I raised this question in mongodb driver Community to which they replied that they are not supporting this version now. so I would like to know if there any specific reason we're still using this older version of MongoDB driver?

fgalan commented 6 years ago

Yes, there is an specific reason for using the legacy driver. In one word: legacy :)

When we started CB development (around 2013), the C++11 wasn't available. Now that C++1 is fully available and stable and the legacy driver is no longer recommended (although still working with at least MongoDB 3.4, see this post) we should migrate to C++11 driver.

However, migration is not an easy task. Although most of the driver operations are encapsulated in a few places (namelly connectionOperations.cpp/h and safeMongo.cpp/h) we extensibly use BSON-related types that, as far as I remember, are also part of the legacy driver.

Migration cannot be done in a single shoot and we need to define a plan, with several intermediate milestones. A first milestone could be to start using the driver in the unit tests code, mostly to gain experience an knowhow on it.

Maybe you want to implement a small proof-of-concept with one unit tests file using the database (the shortest file in test/unittest/mongoBackend) so we can see how it looks like and provide feedback? :) You can have a look to hardening/poc_new_mongo_driver branch if it helps... it was an attemp of include the driver in the CMakeList.txt framework, but I wasn't able to put much time on it and at the end it didn't work.

saurabhjangir commented 6 years ago

@fgalan I have started my investigation regarding this. But I need to know How to start with. My few questions are:

  1. Can I remove current installation of mongodb legacy driver and start with installing latest driver of mongodb driver to know the complexity of the issue and compatibility of the latest driver with current CB?
  2. What I have understood is that the core part which is interacting with mongoDB driver is MongoBackend library( src/lib/mongoBackend). So do I need to should I start with MongoBackend library? I need to know your views and guidance regarding the same. or If any of the above is not a good idea please suggest me how to start with. Thanks :)
fgalan commented 6 years ago

First of all, thank you very much for your proactivity and your will to help with so needed (and at the same time, so complex) task! :)

Let me answer question by question.

But I need to know How to start with

I'd suggest to process the way I mention in my previous comment:

Maybe you want to implement a small proof-of-concept with one unit tests file using the database (the shortest file in test/unittest/mongoBackend) so we can see how it looks like and provide feedback?

First steps would be:

  1. To include the new driver in the CMakefList.txt framework. That is not easy stuff (at least, I didn't manage to make it work) so it would be a very valuable contribution itself :)
  2. Modify the mongo connection initialization logic, i.e. mongoInit()/mongoStart(), to use the new driver. The goal would be to inicialize connection, then use it in the unit test logic, as I suggest above.

Can I remove current installation of mongodb legacy driver and start with installing latest driver of mongodb driver to know the complexity of the issue and compatibility of the latest driver with current CB?

If you refer to documentation, I'd suggest not touching it yet. That would be done when we have some more mature progress.

I'd add in a separate comment some instruction I gather about C++11 installation, in the case it helps.

What I have understood is that the core part which is interacting with mongoDB driver is MongoBackend library( src/lib/mongoBackend). So do I need to should I start with MongoBackend library?

Yes and no :)

Most of the interacting with MongoDB driver is encapsulated in src/lib/mongoBackend library. However, BSON stuff (e.g. BSON() macro, BSONObj, BSONObjtBuilder), which also belong to the legacy driver, are ocassionally used in other libraries.

fgalan commented 6 years ago

I'd add in a separate comment some instruction I gather about C++11 installation, in the case it helps.

Here it is, in the case it may help:

References:

Installation system: Debian 8.10.

Install required CMake 3.2 (Debian 8 comes with con CMake 2.8)

  # sudo apt-get remove cmake cmake-data
  # wget http://www.cmake.org/files/v3.2/cmake-3.2.2.tar.gz
  # tar xf cmake-3.2.2.tar.gz
  # cd cmake-3.2.2
  # ./configure
  # make
  # sudo make install

Install C driver (pre-requirement):

  # wget https://github.com/mongodb/mongo-c-driver/releases/download/1.9.2/mongo-c-driver-1.9.2.tar.gz
  # tar xzfv mongo-c-driver-1.9.2.tar.gz
  # cd mongo-c-driver-1.9.2
  # ./configure --disable-automatic-init-and-cleanup
  # make
  # sudo make install

Install C++11 driver:

  # wget https://github.com/mongodb/mongo-cxx-driver/archive/r3.1.3.tar.gz
  # tar xfvz r3.1.3.tar.gz
  # cd mongo-cxx-driver-r3.1.3/build
  # cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/usr/local -DLIBBSON_DIR=/usr/local -DLIBMONGOC_DIR=/usr/local ..
  # sudo make EP_mnmlstc_core
  # make
  # sudo make install
  # sudo ldconfig
saurabhjangir commented 6 years ago

@fgalan Thanks for the inputs. Currently, I successfully installed latest mongocxx driver on unix system. Tested it, it is working fine. I will update you further once I proceed my work. :)

root@ubuntu16-VirtualBox:~# ./test
{ "_id" : { "$oid" : "5aaabc5a2d8edc750c2cfbf2" }, "hello" : "world" }
saurabhjangir commented 6 years ago

@fgalan I would like to share some changes with you. Also, need suggestions on them.

  1. My Changes in CMakefile.txt are as following: - Adding this to use CXX 11 standards set (CMAKE_CXX_STANDARD 11) - Adding mongocxx and bsoncxx in Dynamic library list
    SET (DYNAMIC_LIBS
     curl
     gnutls
     pthread
     gcrypt
     ssl
     uuid
    +   mongocxx
    +   bsoncxx
    )

    - Including directories for bsoncxx and mongocxx

    
    +include_directories("/usr/local/include/bsoncxx/v_noabi")
    +include_directories("/usr/local/include/mongocxx/v_noabi")
After these changes Broker will use cxx11 standards for compiling the program. Also, we can include headers for latest mongo-cxx-driver in **[mogoBackend/MongoGlobal.cpp](https://github.com/telefonicaid/fiware-orion/blob/master/src/lib/mongoBackend/MongoGlobal.cpp)** file. It won't throw any include directory errors.

But as **auto_ptr** (Used at 14 places in mongoBackend libarary)  is deprecated in cxx 11 standards, It'll raise an error while compiling the code.
I searched for it and found two solutions (Just to avoid compilation Issues). 
One is, auto_ptr in code can be replaced with unique_ptr, which is compatible with cxx 11 standards.
Please have a look at this [question](https://stackoverflow.com/questions/3451099/stdauto-ptr-to-stdunique-ptr).
Other is, we can use `-Wno-deprecated-declarations` in `CMAKE_CXX_FLAGS `Flag which will avoid this warning.

Any of the above will lead to the successful compilation of broker with latest mongo-cxx-driver.

But replacing **auto_ptr** with **unique_ptr** in code is another problem. Some operations might be different in both smart pointers. 

[root@localhost fiware-orion]# ./BUILD_DEBUG/src/app/contextBroker/contextBroker -port 1025 -fg -logLevel debug time=Saturday 17 Mar 00:40:09 2018.651Z | lvl=INFO | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=contextBroker.cpp[1412]:main | msg=Orion Context Broker is running Segmentation fault


Program is crashing because we are using cxx 11 standards and when we initialize mongo client, it is using auto_ptr. Actually I have not used `#ifdef MONGO_CXX_11_DRIVER..<code>.#endif` yet.
Please suggest how to proceed further.
agaldemas commented 6 years ago

Hello, I'm involved in a smart city platform project using Fiware, for metropolis of Nice, in France. I collaborate with @Grenouille06, in this strategic project.

My intuition is many problems regarding Orion crashes with MongoDB >3.4 with some requests (like /v2/types) are mainly due to the use of the old drivers version, may be you already know ;O).

We could have been satisfied with using mongo 3.4, but regarding the target production platform , the MongoDB will be in a docker swarm with clustered architecture, which is not possible in 3.4... So that the support of mongo 3.6.x by Orion is a must for our project, then we are happy, but eager of the move forward.

It's been a long time I put my hands in building C++, but if I can help, tell me how, may be on Docker testing ?

I've a ready to test almost complete Fiware docker based environment with a database (the data are compatible between mongo 3.4 and 3.6=>, once you don't use 3.6 new features, or I can start from empty database). I just need to change the images to use for orion and mongo, so as soon as you have a working build with recent mongo driver, I can test easily everything together (iot agents + orion + cygnus + comet + perseo + wirecloud) !

Thanks in advance for your feedback on my proposal BR.

fgalan commented 6 years ago

@saurabhjangir with regard to auto_ptr issue, if I'm understand correctly, the alternative of using unique_ptr could cause segfault in some cases. Thus, the other alternative (i.e. to use -Wno-deprecated-declarations in CMAKE_CXX_FLAGS) seems better. However, add also a FIXME mark in the CMakeList.txt in order to explain why the new flag is there.

fgalan commented 6 years ago

@agaldemas MongoDB 3.6 issues have its separate issue: https://github.com/telefonicaid/fiware-orion/issues/3070. I agree... it could be related with driver version. Anyway, I'd recommend you to have a look to that issue because it seems that is some scenarios (in particular, when using MongoDB 3.6 from scratch) Orion works.

Thank you for you offer to help with the driver migration :) By the moment, we are in a stage of "initial research" (done mainly thanks to @saurabhjangir ). Once the things get a bit clearer, maybe we can defined specific tasks and you could help with some of them.

Please stay tunned to this issue (we will use it to coordinate among ourselves).

agaldemas commented 6 years ago

Hi, @fgalan OK to wait But to tell you, I've tested with an empty database and mongo 3.6 (mean created from scratch), and the crash with request /v2/types is still there ! I copy @Grenouille06 for him to stay in the loop (with the problem of mongo cluster which require mongo 3.6

fgalan commented 6 years ago

Thanks for the report @agaldemas ! I have re-posted on the #3070 issue (the one about MongoDB 3.6 support, which, in principle, it is an independent issue from this one).

(Probably migrating to C++11 driver is a sufficient condition to make Orion to work with MongoDB 3.6, but not sure if it is also a necessary condition, i.e. there could be solutions -maybe reformulating the query in the aggregation pipeline for the problematic operations- we could mae legacy driver to work with MongoDB 3.6. Further investigation needed).

fgalan commented 5 years ago

After a fruitful interchange of emails with @saurabhjangir (thanks! :), some feedback to take into account:

I think one of the problems of the current codebase is that the usage of the legacy driver is spread around the code, instead of being concentrated in just a single point (or in a small set of points). Although all the DB operations (query, insert, etc.) are isolated in just a couple of modules (connectionOperations.cpp/h and safeMongo.cpp/h) the data structures used by them (BSONObj, BSON, BSONObjBuilder, etc.) are spread around the code. This lack of isolation makes the migration work more complex.

So maybe before starting the migration work itself, we should focus on improving isolation in a reduced set of modules. Then replace the implementation of that modules (.cpp part) with the one corresponding to the new driver. I mean, some rough plan like this:

  1. Move from old C++ standard to C++11 (actually, this is part of an independent issue: https://github.com/telefonicaid/fiware-orion/issues/3509)
  2. Refactor code so at the end nobody uses the driver interface directly. All the usage will be done through “wrapping” modules such as connectionOperations.cpp/h, safeMongo.cpp/h and a new ones if needed (let’s name it bsonStructures.cpp/h by the moment). This can be a large work, so it should be done in small increments (e.g. an initial PR setting up bsonStructures.cpp/h, then a set of small PRs, each one “migrating” from direct usage of the driver to usage through the wrapping modules).
  3. Once the refactor is finished, then re-implement connectionOperations.cpp, safeMongo.cpp and bsonStructure.cpp based in the new C++ driver. Note that connectionOperations.h, safeMongo.h and bsonStructure.h should not change, thus ensuring that the internal interface offered by the modules has not change so no change outside the wrapping modules is needed in other parts of the Context Broker code.

This approach also ensures that if in the future a new migration is needed, it would be easy (just re-implementing again a reduced set of modules but not touching the rest of the code).

fgalan commented 5 years ago

With regards to item 2 this is currently the situation (at master). The following list shows all the source code files in which the string "BSONObj" appears:

$ grep -lR BSONObj src/*
src/lib/rest/StringFilter.cpp
src/lib/rest/StringFilter.h
src/lib/mongoBackend/connectionOperations.h
src/lib/mongoBackend/mongoRegisterContext.cpp
src/lib/mongoBackend/safeMongo.h
src/lib/mongoBackend/mongoRegistrationGet.cpp
src/lib/mongoBackend/mongoRegistrationDelete.cpp
src/lib/mongoBackend/mongoUnsubscribeContext.cpp
src/lib/mongoBackend/mongoRegistrationCreate.cpp
src/lib/mongoBackend/mongoUpdateContextAvailabilitySubscription.cpp
src/lib/mongoBackend/mongoSubCache.cpp
src/lib/mongoBackend/MongoGlobal.cpp
src/lib/mongoBackend/mongoConnectionPool.cpp
src/lib/mongoBackend/mongoUnsubscribeContextAvailability.cpp
src/lib/mongoBackend/compoundResponses.cpp
src/lib/mongoBackend/mongoUpdateSubscription.cpp
src/lib/mongoBackend/location.cpp
src/lib/mongoBackend/MongoCommonRegister.cpp
src/lib/mongoBackend/connectionOperations.cpp
src/lib/mongoBackend/compoundValueBson.h
src/lib/mongoBackend/mongoSubscribeContextAvailability.cpp
src/lib/mongoBackend/MongoGlobal.h
src/lib/mongoBackend/MongoCommonUpdate.cpp
src/lib/mongoBackend/MongoCommonSubscription.cpp
src/lib/mongoBackend/mongoCreateSubscription.cpp
src/lib/mongoBackend/safeMongo.cpp
src/lib/mongoBackend/MongoCommonSubscription.h
src/lib/mongoBackend/mongoGetSubscriptions.cpp
src/lib/mongoBackend/mongoQueryTypes.cpp
src/lib/mongoBackend/dateExpiration.cpp
src/lib/mongoBackend/location.h
src/lib/mongoBackend/mongoSubCache.h
src/lib/mongoBackend/compoundValueBson.cpp
src/lib/apiTypesV2/HttpInfo.cpp
src/lib/apiTypesV2/HttpInfo.h
src/lib/ngsi/ContextElementResponse.h
src/lib/ngsi/Metadata.cpp
src/lib/ngsi/ContextElementResponse.cpp
src/lib/ngsi/Metadata.h
src/lib/ngsi/ContextAttribute.cpp
src/lib/ngsi/ContextAttribute.h

(Maybe there is some "false positive", as BSONObj could appear for instance in a comment but I think the list is pretty accurate. In addition, a similar check could be done with other key classes in the legacy driver like BSONArray, etc. but the result would be pretty much the same).

At the end of task 2 what I hope to have is a reduced set of direct usages to BSONObj, BSONArray, etc. and so on. Ideally, only bsonStructures.cpp and bsonStructures.h. The current usages should be replaced by invocations to the "wrapper" exposed by bsonStructures.h.

fgalan commented 4 years ago

~Related PR https://github.com/telefonicaid/fiware-orion/pull/3613, with a proof-of-concept in one unit test file.~ It will not be needed/used at the end.

fgalan commented 3 years ago

PR #3622 is implementing the actual migration of the driver.

fgalan commented 3 years ago

PR #3622 has been finally merged. Closing issue.