Skip to content

Remove hostnameMaxLen for Mongo URL #6693

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

Merged
merged 2 commits into from
Oct 23, 2020
Merged

Remove hostnameMaxLen for Mongo URL #6693

merged 2 commits into from
Oct 23, 2020

Conversation

markhoward02
Copy link
Contributor

In our Mongo cluster, we have 2 shards equalling 7 hosts that come in as a long comma-delimited list of host names. This hostNameMaxLen of 255 should not apply to a mongo connection string like that:
mongodb://:@MYCLUSTER-shard-00-00.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-00-01.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-00-02.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-00-03.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-02-00.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-02-01.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-02-02.luszb.azure.mongodb.net:27016/dev?ssl=true&authSource=admin

We was using the SRV mongo connection string type which is much shorter:
mongodb+srv://:@MYCLUSTER-luszb.azure.mongodb.net/dev

But we were finding it to be unreliable with Azure's DNS. We rarely change our cluster configuration, so having the long connection string would be more reliable

If these changes should be approached in some other manner, I'd be happy to do that.

In our Mongo cluster, we have 2 shards equalling 7 hosts that come in as a long comma-delimited list of host names.  This hostNameMaxLen of 255 should not apply to a mongo connection string like that:
mongodb://<USER>:<PASSWORD>@MYCLUSTER-shard-00-00.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-00-01.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-00-02.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-00-03.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-02-00.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-02-01.luszb.azure.mongodb.net:27016,MYCLUSTER-shard-02-02.luszb.azure.mongodb.net:27016/dev?ssl=true&authSource=admin

We was using the SRV mongo connection string type which is much shorter:
mongodb+srv://<USER>:<PASSWORD>@MYCLUSTER-luszb.azure.mongodb.net/dev

But we were finding it to be unreliable with Azure's DNS.  We rarely change our cluster configuration, so having the long connection string would be more reliable

If these changes should be approached in some other manner, I'd be happy to do that.
@codecov
Copy link

codecov bot commented May 18, 2020

Codecov Report

Merging #6693 into master will decrease coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6693      +/-   ##
==========================================
- Coverage   93.91%   93.78%   -0.13%     
==========================================
  Files         169      169              
  Lines       12028    12270     +242     
==========================================
+ Hits        11296    11508     +212     
- Misses        732      762      +30     
Impacted Files Coverage Δ
src/vendor/mongodbUrl.js 100.00% <ø> (ø)
src/Adapters/Auth/google.js 92.95% <0.00%> (-7.05%) ⬇️
src/GraphQL/transformers/query.js 83.14% <0.00%> (-6.75%) ⬇️
src/GraphQL/transformers/mutation.js 93.54% <0.00%> (-3.23%) ⬇️
src/GraphQL/parseGraphQLUtils.js 91.66% <0.00%> (-2.28%) ⬇️
src/GraphQL/loaders/usersQueries.js 95.23% <0.00%> (-2.27%) ⬇️
src/triggers.js 92.53% <0.00%> (-0.65%) ⬇️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.92% <0.00%> (-0.61%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 95.86% <0.00%> (-0.47%) ⬇️
src/RestWrite.js 93.82% <0.00%> (-0.19%) ⬇️
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce06f60...8f9da44. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Jul 13, 2020

Would it be better to remove the hostnameMaxLen? Or should we move it to the Parse Server Configurations

@ghost
Copy link

ghost commented Oct 23, 2020

Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page.

Generated by 🚫 dangerJS

@dplewis dplewis marked this pull request as ready for review October 23, 2020 09:27
@dplewis dplewis changed the title Make hostnameMaxLen an environment variable. Remove hostnameMaxLen for Mongo URL Oct 23, 2020
@dplewis dplewis merged commit 12abae8 into parse-community:master Oct 23, 2020
@dplewis
Copy link
Member

dplewis commented Oct 23, 2020

@markhoward02 Thanks for the PR. I went ahead and removed this. You shouldn't have to worry about this issue anymore.

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

Successfully merging this pull request may close these issues.

2 participants