Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conflict between -dbURI and Default -dbTimeout Causes Startup Failure #4496

Closed
goten002 opened this issue Jan 30, 2024 · 8 comments
Closed
Labels
Milestone

Comments

@goten002
Copy link
Contributor

Bug description
Encountered a startup failure with Fiware Orion version 3.11.0 due to a conflict between the -dbURI parameter and the default value of -dbTimeout. The issue arises when the -dbURI parameter is used without explicitly setting -dbTimeout to zero.

  • Orion version: 3.11.0
  • MongoDB version 4.4
  • CLI parameters: -dbURI mongodb://mongodb:27017 -logLevel DEBUG
  • Docker Image used: fiware/orion:3.11.0

How to reproduce it

  1. Configure Orion with the -dbURI parameter without specifying -dbTimeout.
  2. Attempt to run the Orion container.
  3. Observe the startup failure with the error message indicating a conflict between -dbURI and other database parameters.
Invalid Command Line Options: -dbURI cannot be combined with -dbhost, -rplSet, -dbTimeout, -dbuser, -dbAuthMech, -dbAuthDb, -dbSSL and -dbDisableRetryWrites.

Expected Behavior
Orion should start successfully using the -dbURI parameter without needing to explicitly set -dbTimeout to zero, as -dbTimeout has a default value.

Investigation and Analysis
On further investigation, it was found that the composeMongoUri function in mongoConnectionPool.cpp checks if timeout > 0 to return an error. The default value of dbTimeout is 10000, as seen in contextBroker.cpp, which seems to be causing the issue.

Suggested Fix
It appears that the check for timeout > 0 in the condition within composeMongoUri is unnecessary when -dbURI is used. A proposed solution would be to modify the conditional check to exclude timeout when dbURI is provided. This would prevent the conflict when the default dbTimeout value is used with -dbURI.

Code Reference
The relevant code is located at:

Additional Context
This issue may impact users who are transitioning to the new -dbURI parameter and are unaware of the need to explicitly set -dbTimeout to zero.

@goten002
Copy link
Contributor Author

While working on resolving the dbTimeout and -dbURI conflict (Issue #4496), I noticed an anomaly concerning the ORION_MONGO_TIMEOUT environment variable in a Docker environment.

Setup:
I used the following Docker Compose configuration:

version: '3.9'
services:
  orion:
    image: fiware/orion:3.11.0
    depends_on:
      - mongodb
    ports:
      - "1026:1026"
    environment:
      - ORION_MONGO_URI=mongodb://mongodb:27017
      - ORION_MONGO_TIMEOUT=0
      - ORION_LOG_LEVEL=DEBUG
  mongodb:
    image: bitnami/mongodb:4.4
    ports:
      - "27017:27017"

Observation
Despite setting ORION_MONGO_TIMEOUT to various values (including 0), the logs always indicate that ORION_MONGO_TIMEOUT is set to 80. This can be seen in the log output captured with logEnvVars:

op=contextBroker.cpp[972]:logEnvVars | msg=env var ORION_MONGO_URI (-dbURI): mongodb://mongodb:27017
op=contextBroker.cpp[988]:logEnvVars | msg=env var ORION_MONGO_TIMEOUT (-dbTimeout): 80
op=contextBroker.cpp[1180]:main | msg=Orion Context Broker is running
op=mongoConnectionPool.cpp[536]:mongoConnectionPoolInit | msg=Connected to mongodb://mongodb:27017 (dbName: orion, poolsize: 10)
op=contextBroker.cpp[1320]:main | msg=Startup completed

The functionality does not seem to be affected, as the actual value of ORION_MONGO_TIMEOUT appears to be correctly applied. However, the logging output is misleading, potentially causing confusion about the configuration in use.

I'm documenting this here for awareness and to check if it's related to the changes made for Issue #4496, or if it should be addressed separately.

@fgalan
Copy link
Member

fgalan commented Jan 30, 2024

Thanks for so detailed report and related PR #4497! :)

Let me some time to review it and I'll provide feedback.

@fgalan
Copy link
Member

fgalan commented Feb 6, 2024

Probably the simplest solution is to change the -dbTimeout default from 10000 to 0.

On the one hand, this will "pass" the || timeout > 0 condition if -dbUri is used

On the other hard, this will cause that connectTimeoutMS= not being added to the composition of the mongo URI (if -dbUri is not used). Although the composed URI will not be the same that with the current 10000 default (i.e connectTimeoutMS= the actual effect is the same, taking into account from https://www.mongodb.com/docs/manual/reference/connection-string/#timeout-options

imagen

and from the cited driver documentation, in our case (Mongo C driver) from http://mongoc.org/libmongoc/current/mongoc_uri_t.html

imagen

Thus, the default in the case of not using connectTimeoutMS= is the same.

@goten002 what do you think? Could you adapt your PR #4497 in that way? (the PR would simplify a lot, in fact)

@fgalan
Copy link
Member

fgalan commented Feb 6, 2024

I'm documenting this here for awareness and to check if it's related to the changes made for Issue #4496, or if it should be addressed separately.

Thanks for raising it. Solved in PR #4501

@goten002
Copy link
Contributor Author

goten002 commented Feb 6, 2024

@goten002 what do you think? Could you adapt your PR #4497 in that way? (the PR would simplify a lot, in fact)

Thank you very much for the thorough analysis and the proposed solution. I agree that changing the default value of -dbTimeout from 10000 to 0 is the optimal approach, especially considering the Mongo C driver's default behavior effectively mirrors what we intended with the original 10000 ms default. This change simplifies the implementation and avoids the complications associated with using negative values. I appreciate your help and guidance on this matter and will proceed to adjust my PR #4497 accordingly.

fgalan pushed a commit that referenced this issue Feb 7, 2024
* FIX -dbTimeout conflict with -dbURI

* Adjust timeout handling and fixing syntax error

* Simplify dbTimeout Handling by Setting Default to 0

* Update CHANGES_NEXT_RELEASE for -dbTimeout Default Value Fix

* Adjust Test Expectations for New -dbTimeout Default
@fgalan
Copy link
Member

fgalan commented Feb 7, 2024

I would say that after merging PRs #4497 and PR #4501 this issue could be closed. @goten002 what do you think?

Thank you for your outstanding contribution! If you want to continue contributing to Orion I'd suggest to have a look to help wanted labeled issues or to the Orion roadmap, for more challenging issues.

@fgalan fgalan added this to the 3.12.0 milestone Feb 7, 2024
@goten002
Copy link
Contributor Author

goten002 commented Feb 7, 2024

Thank you @fgalan, for your guidance and support throughout this process. I agree that once PRs #4497 and #4501 are merged, the issue can be closed.

@fgalan
Copy link
Member

fgalan commented Feb 7, 2024

Btw, this issue has made me think it is a good idea to deprecated the old CLIs. This PR has been created about it #4505.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants