[GODRIVER-1557] Improve SetCompressors documentation Created: 06/Apr/20 Updated: 28/Oct/23 Resolved: 14/Apr/20 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | Connections, Options & Configuration |
| Affects Version/s: | None |
| Fix Version/s: | 1.4.0 |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Ed Pelc | Assignee: | Divjot Arora (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Backwards Compatibility: | Minor Change | ||||
| Description |
|
UPDATED DESCRIPTION: The SetCompressors documentation makes it seem like there is user action required to enable compression server-side, but this is not true because the server has compression enabled by default. There's some small docs changes we can make here to improve the wording.
ORIGINAL DESCRIPTION: I just noticed that you can enable compression on connections. The docs make it appear as if this requires special server config to enable but `mongod` actually has all compressors enabled by default even though the older ones are prefferred over newer ones.
I propose pending any potential perf or other issues related to the current implementation that we match the default server config and enable compressors by default. I think the tests might need to be changed to explicitly disable compression though. The readme says that is their default and I'm not sure how much they depend on that since they are testing driver code.
ie change from empty slice to `[]string{"snappy","zstd","zlib"}`
https://docs.mongodb.com/manual/reference/program/mongod/#cmdoption-mongod-networkmessagecompressors
https://pkg.go.dev/go.mongodb.org/mongo-driver/mongo/options?tab=doc#ClientOptions.SetCompressors
Any thoughts on if this would break anything? I would think it'd be a good performance boost in most cases.
|
| Comments |
| Comment by Githook User [ 14/Apr/20 ] |
|
Author: {'name': 'Divjot Arora', 'email': 'divjot.arora@10gen.com', 'username': 'divjotarora'}Message: |
| Comment by Ed Pelc [ 14/Apr/20 ] |
|
--
Just saw your PR and it is much nicer. Thank you! |
| Comment by Divjot Arora (Inactive) [ 14/Apr/20 ] |
|
Docs changes: https://github.com/mongodb/mongo-go-driver/pull/367 |
| Comment by Divjot Arora (Inactive) [ 07/Apr/20 ] |
|
epelc@greatcloak.com I assume you're talking about the docs for SetCompressors (https://pkg.go.dev/go.mongodb.org/mongo-driver/mongo/options?tab=doc#ClientOptions.SetCompressors). We try not to mention server defaults in our documentation, as those are not guaranteed to stay the same between server versions. For example, zstd was not added until 4.2. Instead, we link to pages on docs.mongodb.com, as those are kept up to date by our Docs team. That being said, I understand that the current SetCompressors docs make it seem like explicit action is needed to enable compression server-side. To address that, I propose that we remove the line "To use compression, it must be enabled on the server as well" and change the line that links to the server docs to "See https://docs.mongodb.com/manual/reference/program/mongod/#cmdoption-mongod-networkmessagecompressors for information about how compression settings can be tuned on the server." The docs at that link specify that all compressors are enabled by default server-side. Thoughts? |
| Comment by Ed Pelc [ 07/Apr/20 ] |
|
Thanks for the thorough reply! I don't really keep up with the other drivers but it's really good to hear there is a goal to unify them all. I agree with it not being a perf upgrade 100% of the time. Maybe the docs could be clarified to say that the server does have compression enabled by default though? It looks like compression has been enabled by default(server side) since it came out in 3.4. |
| Comment by Divjot Arora (Inactive) [ 07/Apr/20 ] |
|
We recognize that correctly choosing the optimal compression settings can be tough and that the driver documentation for this can be improved. However, enabling compression by default isn't a change that we want to unilaterally make in the Go driver. There is an ongoing discussion about how we can reliably do this across all drivers. This is more complex than it may seem, given that not all drivers have native libraries for all compression types and compression can actually decrease application performance in some setups and workloads. We'll decide what to do for Go depending on that discussion. In the meantime, we'd love to hear any suggestions you might have about documentation. There are a large number of Client options and we realize that important information like this can get lost in the mix. I'm going to leave this ticket open for now and we might be able to re-purpose it as a documentation improvement if you have any suggestions. – Divjot |