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

Sync optimizations #6234

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open

Sync optimizations #6234

wants to merge 43 commits into from

Conversation

snej
Copy link
Contributor

@snej snej commented May 10, 2023

CBG-2952

Improvements to the sync code in 'db' and to 'go-blip', to optimize replication by reducing the amount of memory and goroutines used.

  • Significant changes to concurrency in go-blip; see Improve concurrency: fewer goroutines go-blip#62
  • Added blipRevSender which manages the fetching of doc revisions and sending of 'rev' messages. It throttles this to a limited rate so that the number being written to the socket and their total byte size don't exceed a limit.
  • Fixed some code that sent BLIP messages and blocked waiting for a reply; instead it uses the new Message.OnReply method to schedule a callback on the thread-pool.
  • Increased the maximum number of concurrent outgoing 'changes' messages from 2 to 4.
  • Made outgoing 'changes' messages have Urgent delivery so they arrive sooner.

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

  • Link upstream PRs
  • Update Go module dependencies when merged

Integration Tests

gregns1 and others added 11 commits May 9, 2023 15:03
* CBG-2894: Reject user auth when channel threshold is over 500 in serverless mode

* fix panic where authetciator was needed and it wasn't availible

* linter issue

* linter issue again

* remove extra methods off interface

* pass user into function

* rebase

* ensure 500 code is retruned for http error added

* updates based off comments

* fix panic

* updates based off comments

* updates based off dicussion yesterday

* lint error

* updates based of comments
* Add stats for transferred of bytes data ReplicationBytesReceived and ReplicationBytesSent
* Stats are updated when a BlipSyncContext exits and at a threshold set by DatabaseContextOptions.BlipStatsReportingInterval (set to thirty seconds arbitrarily to see perf impact).
* Use sgMgrEventHandlers context to add cancellation handling for EOF feed errors

* Move all importListener.Stop cbgtContext code into CbgtContext.Stop()
…hanges feed (#6243)

* CBG-2853 Add requestPlus option for changes feeds

Adds requestPlus option for changes feeds.  When set, changes feeds will loop until the cached sequence (via DCP) is greater than the database sequence at the time the changes request was issued.

requestPlus can be enabled for non-continuous changes requests in one of three ways:
- by setting request_plus=true on a REST API changes call
- by setting the requestPlus property to "true" on a subChanges message
- by setting "changes_request_plus":true in the database config (default=false)

The request setting is given priority - if not set on a request, the value will fall back to the database config value.

Required minor refactoring of how options.Wait was used in changes.go, to support use of requestPlus and longpoll together.  No functional changes to longpoll if requestPlus is not set.

* Update docs for request_plus changes parameter.

* lint fixes
@snej snej requested a review from bbrks May 15, 2023 20:11
adamcfraser and others added 18 commits May 15, 2023 15:55
* 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-3024 Make sure import feed uses checkpoints
* 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.
- Increase maxInFlightChangesBatches from 2 to 4
- Send `changes` requests as Urgent, as LiteCore does
Updated go-blip, with new API for registering & calling request handlers.
snej added 2 commits May 30, 2023 15:19
`blipRevSender` manages a list of `revToSend` structs identifying revisions to
be sent as `rev` messages, and sends them at a limited rate so that the number
being written to the socket and their total size don't exceed a limit.
The struct has a Mutex in it now so it shouldn't be copied.
torcolvin and others added 7 commits May 30, 2023 22:57
* CBG-3028: fixes for failing CE tests

* remove print line

* updates off comment
…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
@snej
Copy link
Contributor Author

snej commented Jun 5, 2023

Still awaiting performance testing by perf team: CBPS-1153. Review is blocked on this.

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.

None yet

5 participants