Conversation
Redocly previews |
gregns1
changed the title
gregns1
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds supportability/observability signals around metadata migration by exposing additional runtime state via admin endpoints, adding a new migration “abandoned runs” stat, and updating OpenAPI + tests to cover the new fields.
Changes:
- Add
metadata_store_modeto/_statusto indicate whether dual-store fallback reads are active/inactive. - Extend
/_cluster_infobucket payload with cached bootstrap target and bucket-level metadata migration status. - Add
abandoned_runsmetadata migration stat for “hit retry ceiling” failures, plus supporting tests/docs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rest/utilities_testing_resttester.go | Adds a RestTester helper to wait for /_metadata_migration to reach a target state. |
| rest/metadatamigrationtest/metadata_migration_test.go | Asserts /_cluster_info includes bootstrap_target for the bucket after migration flow. |
| rest/adminapitest/admin_api_test.go | Adds admin API tests for migration_status in /_cluster_info and metadata_store_mode in /_status. |
| rest/admin_api.go | Implements new /_status + /_cluster_info response fields and wires them into handlers. |
| docs/api/components/schemas.yaml | Updates OpenAPI schemas for new /_status and /_cluster_info fields and adds new migration status schemas. |
| db/background_mgr_metadata_migration.go | Increments new “abandoned runs” stat when the bounded pass loop gives up. |
| db/background_mgr_metadata_migration_test.go | Updates test setup to include DbStats needed for the new stat path. |
| base/stats.go | Adds abandoned_runs to migration stats and registers/unregisters it. |
| base/stats_descriptions.go | Adds description text for the new abandoned_runs stat. |
| base/rosmar_cluster.go | Implements CachedBootstrapTargets() for Rosmar bootstrap connection. |
| base/dual_metadata_store.go | Introduces MetadataStoreMode and helper to classify metadata store mode for /_status. |
| base/bootstrap.go | Extends BootstrapConnection interface and implements cached bootstrap target exposure for CouchbaseCluster. |
| func (cc *CouchbaseCluster) CachedBootstrapTargets() map[string]string { | ||
| targets := make(map[string]string) | ||
| cc.bucketBootstrapTargets.Range(func(key, value any) bool { | ||
| if s := value.(bucketBootstrapTarget).String(); s != "" { |
There was a problem hiding this comment.
I think we should type check on s, ok := value.(bucketBootstrapTarget)
Also, do you think we should throw an assertion error if s == ""? Should we return unknown, so we know it isn't just missing?
There was a problem hiding this comment.
Will return unknown for missing. Updates docs to reflect this too. I think the type asserting everywhere is a bit messy and not even sure what to do in event of type assert failing (although that shouldn't happen). So i made a SyncMap wrapper to allow generics to make it type safe.
| rest.RequireStatus(t, resp, http.StatusOK) | ||
| err := base.JSONUnmarshal(resp.BodyBytes(), &clusterInfoResponse) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "system_mobile", clusterInfoResponse.Buckets[rt.Bucket().GetName()].BootstrapTarget) |
There was a problem hiding this comment.
I think we can add this into existing tests but I'd love to see a case where it returns either "" (or omited) and also "default"
There was a problem hiding this comment.
Add assertion to test for _default case. Separate test made for omitted case.
CBG-5413
_cluster_infoendpoint - this tells us the cached bootstrap-doc location for each bucket_cluster_infoendpoint - tracks the per-bucket metadata-migration lifecycle_statusendpoint - this is to tell us if fallback reads are active or not on the metadata store. Left empty if MetadataStore is not usedThis is all info I think will be useful in sgcollect for debugging purposes.
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiDependencies (if applicable)
Integration Tests