-
Notifications
You must be signed in to change notification settings - Fork 141
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
Make the all_docs endpoint with Fields faster #3547
Conversation
When requesting all_docs for a lot of io.cozy.files with Fields, the response can be quite slow (a few seconds). Part of that is CouchDB and transfering the bytes, and we can't do anything about that, but there is also another part that is deserializing JSON, keeping only the required fields, and reserializing to JSON. And it looks like this part is taking way much longer that we would have expected. In this commit, we are trying to make things faster by replacing the full JSON parser by just a tokenizer (and doing some low-level stuff). I have added unit tests, but the code is tricky, so I'm less confident than usual on the risk of a regression. And we need to deploy on servers to really see the performance boost (or lack of).
|
||
log := logger.WithDomain(db.DomainName()).WithNamespace("couchdb") | ||
if log.IsDebug() { | ||
log.Debugf("request: %s %s %s", method, path, "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the empty string at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function was copied and adapted from couchdb.makeRequest
. The last string is for the body of the request, and we don't any body here. I've kept the same log format, but I agree it's a bit weird to have an empty string here.
fields []string | ||
skipDDoc bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be worth documenting these 2 attributes, especially the second one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are config, ie things passed by the caller, and I think that the 2 methods that manipulates them have explicit comments: NewAllDocsFilter
and SkipDesignDocs
. Do you think we should still comment the 2 attributes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to see documented fields rather than having to look for their usage to understand what they mean. But since they're private attributes maybe I'm looking at it the wrong way and I should focus on the public API.
Co-authored-by: Erwan Guyader <[email protected]>
Co-authored-by: Erwan Guyader <[email protected]>
Co-authored-by: Erwan Guyader <[email protected]>
Co-authored-by: Erwan Guyader <[email protected]>
Co-authored-by: Erwan Guyader <[email protected]>
Co-authored-by: Erwan Guyader <[email protected]>
When requesting all_docs for a lot of io.cozy.files with Fields, the response can be quite slow (a few seconds). Part of that is CouchDB and transfering the bytes, and we can't do anything about that, but there is also another part that is deserializing JSON, keeping only the required fields, and reserializing to JSON. And it looks like this part is taking way much longer that we would have expected.
In this commit, we are trying to make things faster by replacing the full JSON parser by just a tokenizer (and doing some low-level stuff). I have added unit tests, but the code is tricky, so I'm less confident than usual on the risk of a regression. And we need to deploy on servers to really see the performance boost (or lack of).