Refactor document code, and avoid [un]marshaling doc bodies #6108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
snej
force-pushed
the
refactor-document
branch
3 times, most recently
from
5df9a94
to
f811b49
Compare
snej
force-pushed
the
refactor-document
branch
from
f811b49
to
d07305f
Compare
snej
changed the title
- JSONExtract now uses a callback to delegate the logic about which keys are special/reserved. - implemented JSONExtract for CE - DocumentRevision calls JSONExtract, handles special properties.
# Conflicts: # rest/replicatortest/replicator_test.go
Body changed to UnmarshalBody. Removed some usages of both methods, working with JSON data instead.
- RevisionCache.GetRevision no longer returns a Body - Removed body field from revCacheValue struct
…replication protocol (#6242)
|
@snej You indicate that there are no functional changes in this PR - how does that apply to the changes around unmarshalling document bodies? Can you summarize the changes being made to that functionality? |
|
@adamcfraser In this PR the document bodies are still unmarshaled when passed to JS (Otto). That will change with the V8 PR, at least when V8 is enabled; there we pass the JSON as a string and unmarshal it in JavaScript which is more efficient. Otherwise, incoming doc bodies are still validated to make sure they don't contain illegal/reserved properties, but that's done without unmarshaling. The JSON does get parsed, but without converting it to Go objects which is the expensive part. |
* remove cached buckets * Add refcounting for parallel calls to get buckets
* Update waiting error message from generic message Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>
* Tweak TestIncrCounter to cover non-equal `def` and `amt` values * Allow zero value in `amt` for Collection.Incr - will write `def` to bucket if counter does not exist
* CBG-2983 Close cbgt agents on database close Manager.Stop() doesn’t close cbgt’s gocb agents - partly because the fts use case is to have one manager that exists for the lifetime of the process. For SG’s usage, where manager lifecycle is bound to a database, we need to shut down these agents when we close the database/importListener. * Improve inline documentation for CloseStatsClients call
* CBG-3001 Avoid bucket retrieval error during OnFeedClose OnFeedError was checking for bucket existence to determine whether to call NotifyMgrOnClose(). This handling isn't necessary for SG, as we want database close to handle shutdown of the import feed (via importListener.Stop()) in the case of a deleted bucket.
# Conflicts: # documents/revision_cache_interface.go
…6258) * CBG-2793: attachment compaction code erroneously sets failOnRollback * spelling errors and test logging removal + lint error * updates for comments and try fix race condition * linter error fix * changes for jenkins * updates to add handling for cleanup phase * remove logging line * fix race in Jenkins * updates to fix collections issue * updates based off comment * updates * updates to address complexity comments
|
Created a perf testing task CBPS-1176 for this. |
# Conflicts: # go.mod # go.sum
|
Perf results show a decent ~5% improvement in throughput (and accompanying reduction in CPU) - we should go ahead and review/merge based on this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CBG-2951
Moved most of the Document, Revision and RevTree code, and a lot of Attachment stuff, out of the db package into a new
documentpackage. (I'm not wild about that name, butdocwould conflict with the very common parameter namedoc, anddocsis already taken as a directory.) Two reasons for this:Document, protect their private fields from being groped by other code in the db package. There's a fair bit of this going on with the_bodyand_rawBodyfields.)dbpackage is really big and it would be nice to have smaller packages, IMHO, just for readability and code navigation.To reduce the number of diffs, I created a file
db/doc_model.gothat creates aliases for many of the types and constants that were moved. I also added an alias for the functionParseRevIDbecause there are so damn many calls to it.To cope with the code in db that touched Document's
_bodyand_rawBodyfields, I added methodsPeekBody,PokeBody,PeekRawBody,PokeRawBody. These can be removed after the code that calls them is fixed.There are no functional changes in this PR; everything should behave exactly as before.
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiDependencies (if applicable)
Integration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/000/