Skip to content

CBG-5260: remove panic call in rev cache Put#8237

Merged
bbrks merged 5 commits into
mainfrom
CBG-5260
May 11, 2026
Merged

CBG-5260: remove panic call in rev cache Put#8237
bbrks merged 5 commits into
mainfrom
CBG-5260

Conversation

@gregns1
Copy link
Copy Markdown
Contributor

@gregns1 gregns1 commented May 6, 2026

CBG-5260

Removes explicit panic call in Put on revision cache. Adds validation on document revision and if invalid pass error up the stack for Put and Upsert methods.

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

Copilot AI review requested due to automatic review settings May 6, 2026 14:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the panic in the revision cache Put path by introducing structural validation for DocumentRevision and propagating errors from Put/Upsert up through the revision cache layers. It also updates callers/tests to handle the new error-returning signatures and adjusts memory-accounting tests to reflect history bytes being counted.

Changes:

  • Change RevisionCache/collection wrappers/orchestrator/sharded cache Put and Upsert to return error, and update call sites to handle it.
  • Add DocumentRevision.IsValid() validation and return a redactable error instead of panicking when required fields are missing.
  • Update revision/delta cache tests to use valid history/body inputs and to assert the revised memory byte accounting; add a new test for invalid revision validation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
rest/replicatortest/replicator_test.go Updates test to assert Upsert returns no error after signature change.
db/revision_cache_test.go Updates tests for new Put/Upsert error returns and revised memory accounting; adds invalid-revision test.
db/revision_cache_orchestrator.go Propagates Put/Upsert errors and only triggers eviction on success.
db/revision_cache_lru.go Removes panic, adds IsValid and returns validation errors from Put/Upsert.
db/revision_cache_interface.go Updates RevisionCache interface and collection wrapper methods to return errors.
db/revision_cache_bypass.go Updates bypass implementation to match new interface (returns nil error).
db/delta_cache_test.go Adjusts delta cache test inputs/expectations to satisfy new validation and byte accounting.
db/crud.go Handles revision cache insertion errors by logging (without failing the write).

Comment thread db/revision_cache_test.go
Comment thread db/revision_cache_test.go Outdated
Comment thread db/revision_cache_test.go Outdated
Comment thread db/revision_cache_test.go Outdated
Copy link
Copy Markdown
Member

@bbrks bbrks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly OK - think we should have better error reporting if docs end up in this invalid state

Comment thread db/crud.go Outdated
Comment thread db/revision_cache_lru.go Outdated
Comment thread db/revision_cache_test.go Outdated
@bbrks bbrks merged commit 8ce49b3 into main May 11, 2026
50 checks passed
@bbrks bbrks deleted the CBG-5260 branch May 11, 2026 13:44
RIT3shSapata added a commit that referenced this pull request May 15, 2026
Squashed commit of the following:

commit 8d1f7e9
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Thu May 14 15:15:55 2026 +0100

    CBG-5386: Grafana generator: Stamp `"version": 1` (#8274)

commit f3e5570
Author: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com>
Date:   Thu May 14 12:46:37 2026 +0100

    CBG-5381: fix resync regenerate sequences set sync info no-op (#8267)

commit f277042
Author: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com>
Date:   Thu May 14 12:44:08 2026 +0100

    CBG-5249: collect value of GOMEMLIMIT in sgcollect (#8269)

commit 68fbc4c
Author: Tor Colvin <tor.colvin@couchbase.com>
Date:   Wed May 13 20:07:59 2026 -0400

    Move a db scoped function from DatabaseCollection (#8248)

commit 8173a32
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Wed May 13 00:21:18 2026 +0100

    Improve agent instructions (#8260)

commit d440374
Author: Tor Colvin <tor.colvin@couchbase.com>
Date:   Tue May 12 12:04:17 2026 -0400

    CBG-5376 skip test temporarily (#8265)

commit d17c077
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Tue May 12 14:30:16 2026 +0100

    CBG-5298: Cleanup for GetUser (#8259)

commit a3952d3
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Tue May 12 11:23:32 2026 +0100

    Bump golang.org/x modules (#8255)

commit fbd2d74
Author: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com>
Date:   Tue May 12 10:33:31 2026 +0100

    CBG-5365-followup: fix escaping issue (#8256)

commit 3452ce2
Author: Tor Colvin <tor.colvin@couchbase.com>
Date:   Mon May 11 11:45:54 2026 -0400

    Fix issues from go vet: (#8251)

commit f419c1e
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Mon May 11 16:45:40 2026 +0100

    CBG-5266: Prevent downgrades across cluster compatibility versions (#8235)

    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

commit d09f0fd
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Mon May 11 16:41:09 2026 +0100

    CI: upload rendered test logs as artifacts on failure (#8246)

    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

commit cb95e1d
Author: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com>
Date:   Mon May 11 16:29:42 2026 +0100

    CBG-5365: escape . characters for channel removal macro expansion (#8241)

commit 51a86de
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Mon May 11 16:24:26 2026 +0100

    CBG-5290: Wire up ctx into sg-bucket apis (#8240)

    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

commit 8ce49b3
Author: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com>
Date:   Mon May 11 14:44:56 2026 +0100

    CBG-5260: remove panic call in rev cache Put (#8237)

commit 455e037
Author: Tor Colvin <tor.colvin@couchbase.com>
Date:   Mon May 11 09:13:29 2026 -0400

    Fix TestImportPartitionsServerless in CE mode (#8250)

commit c049b7c
Author: Ben Brooks <ben.brooks@couchbase.com>
Date:   Mon May 11 10:01:40 2026 +0100

    CBG-5291: CAS-safe Update on dual MetadataStore wrapper (#8238)

    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

commit 6890541
Author: Tor Colvin <tor.colvin@couchbase.com>
Date:   Fri May 8 18:50:29 2026 -0400

    CBG-4920 Fix test flake: TestActiveReplicatorHLVConflictCustom (#8247)

commit 0c4ca83
Author: Tor Colvin <tor.colvin@couchbase.com>
Date:   Fri May 8 16:16:39 2026 -0400

    CBG-5175: resync - purge checkpoints if needed (#8233)

    Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

commit eb3acba
Author: Tor Colvin <tor.colvin@couchbase.com>
Date:   Fri May 8 11:10:03 2026 -0400

    CBG-4496 add regression test for SetRaw/AddRaw for rosmar (#8239)

    Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>

commit 24da9f1
Author: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com>
Date:   Fri May 8 14:46:41 2026 +0100

    CBG-5355: fix panic for on demand get when ErrImportCancelled is returned from import (#8225)
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.

3 participants