Repository navigation
chore(trunk): run lint on Go 1.27.3 with golangci-lint 2.15.0 - #9868
Merged
Merged
Conversation
go.mod has required go 1.27.3 since #9864, but trunk still ran the go 1.27.0 runtime with golangci-lint 2.13.1. CI auto-switches to the 1.27.3 toolchain, which writes export data version 5, and the x/tools v0.49.0 importer inside golangci-lint 2.13.1 only reads up to version 4. Package x then fails to load, so every promoted x.SafeMutex method reports as undefined, and trunk flags whichever of those land in a changed hunk: s.orc.RLock undefined (type *Oracle has no field or method RLock) golangci-lint 2.15.0 is built on x/tools v0.52.0 and reads the new export data; locally it reports no new issues on #9747 under the default, integration and integration2 build tags. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: updating the Go runtime and golangci-lint versions used by trunk. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
3 tasks done
matthewmcneely
pushed a commit
that referenced
this pull request
Oct 27, 2026
**Description**
`dgraph restore -z` (offline restore) never bumps Zero's uid or
namespace lease.
`RunOfflineRestore` ran the map phase with `if _, err :=
RunMapper(...)`, which discarded the
`mapResult` that carries `maxUid` and `maxNs`. It then returned
`LoadResult{Version: ...}` under
`// TODO: Fix this return value.`, so `MaxLeaseUid` and `MaxLeaseNsId`
were always 0. `backup/run.go`
prints `Restore max uid: 0`, and its `leaseID` helper treats 0 as
"backup taken on an empty DB" and
skips the `AssignIds` call. Only the timestamp lease is bumped.
As a result, after an offline restore of a non-empty backup into a fresh
cluster, Zero's `maxUID`
stays 0. The first new node can be assigned a uid that already belongs
to a restored node (we saw
`0x1`), and the new predicates merge into that node. Nothing reports an
error, and restored data
reads back correctly; the corruption only appears on the next write. The
online `/admin` restore
is not affected, because it bumps both leases from the same mapper
result.
With ACL enabled, the colliding uids belong to ACL nodes. In a local
v25.5.0 reproduction (a backup taken
with `--acl`, offline-restored with `-z` into a fresh cluster), Zero
reported `maxUID: 0` and the first new
nodes were assigned `0x1` to `0x4`. Those uids hold the guardians group,
groot, another user and a rule, and
each merged node then carried both the application type and its ACL type
(for example
`["Item", "dgraph.type.Group"]` on the guardians node). The same backup
restored through `/admin` assigned
new uids above the backup's maximum.
The fix keeps the mapper's `maxUid` and `maxNs` across groups and
returns them in the `LoadResult`.
A second commit changes how `dgraph restore` hands those values to Zero.
It sent them as a plain
`AssignIds` request, which leases that many *more* ids instead of
raising the lease to that value. On a
fresh Zero the two are the same. But every further restore against the
same Zero moved the lease again,
by the backup's max each time. The online restore's bump (`Bump: true`,
with "Nothing to be leased"
treated as a no-op) now lives in `worker.BumpLease`, and both restore
paths use it.
Regression test: `TestRunOfflineRestoreReportsMaxLease`
(`worker/offline_restore_test.go`) writes a
real full backup through `BackupProcessor` (uids up to `0x2711` across
namespaces 0 and 2). It checks
that `RunMapper` sees both maxima, then that `RunOfflineRestore` reports
them. Before the fix it fails:
```
Error: "0" is not greater than or equal to "10001"
Error: "0" is not greater than or equal to "2"
--- FAIL: TestRunOfflineRestoreReportsMaxLease
```
After the fix it passes, as do `go test ./worker/ -run
'Restore|Backup|Manifest'`. The existing
systest backup suites call `RunOfflineRestore` but never read the lease
fields, which is why this
went unnoticed.
`TestBumpLeaseIsIdempotent` (`worker/lease_test.go`, `integration` tag,
run with
`make test-integration TEST=TestBumpLeaseIsIdempotent`) bumps a real
Zero's uid lease, then bumps it again
to the same value and to a lower one, and checks that the lease does not
move. It passes. With
`BumpLease` temporarily changed back to the old plain lease, it fails on
the repeated bump:
```
Error: "2506" is not less than "1103"
--- FAIL: TestBumpLeaseIsIdempotent
```
Docker integration suite `systest/backup/filesystem`, run locally on the
first commit, before rebasing onto
#9864 and #9868, both chore commits (each test by name, since
`-failfast` stops the package at the
first failure):
- `TestBackupFilesystem` passes. It runs `RunOfflineRestore` over a full
backup and four
incrementals.
- `TestBackupOfOldRestore`, `TestBackupSummaryManifest` and all six
`TestListBackups*` tests pass.
- `TestRestoreOfOldBackup/backup_of_21.03` fails by timing out in
`WaitForRestore`. It fails the same
way on unmodified `main` (b9d2c9c). It uses the online restore, which
this change does not touch.
Not run locally: the other backup suites (minio, encryption, nfs,
multi-tenancy, advanced-scenarios),
and an end-to-end `dgraph restore -z` followed by a check of Zero's
`/state`. That end-to-end path
was how we first found this, on v25.5.0, where Zero reported `maxUID: 0`
after the restore.
**Checklist**
- [x] The PR title follows the
[Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/#summary) syntax,
leading
with `fix:`, `feat:`, `chore:`, `ci:`, etc.
- [x] Code compiles correctly and linting (via trunk) passes locally
- [x] Tests added for new functionality, or regression tests for bug
fixes added as applicable
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- codesmith:footer -->
---
<a
href="https://app.blacksmith.sh/dgraph-io/codesmith/dgraph/pr/9869?autoLogin=true&ref=codesmith_pr_footer"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v3.svg"><img
alt="View with [code]smith"
src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3-3x.png"
width="162" height="28"></picture></a> <a
href="https://backend.blacksmith.sh/track/enable-autofix?expires=1795451831&installation_model_id=8575&pr_number=9869&ref=codesmith_pr_footer&repository=dgraph-io%2Fdgraph&return_to=https%3A%2F%2Fgithub.com%2Fdgraph-io%2Fdgraph%2Fpull%2F9869&signature=4e6d921a95550c19003351bd5921d7ba2eaec4d6bdf9b5e3baaa830edd463ca7"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img
alt="Autofix with [code]smith"
src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark-3x.png"
width="190" height="28"></picture></a>
<sup>Need help on this PR? Tag <code>@codesmith-bot</code> with what you
need.</sup>
<!-- codesmith:autofix:disabled -->
<!-- /codesmith:footer -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* Offline restore results now include the highest UID and namespace
values found across restored groups, along with the backup’s read
timestamp.
* Restore operations now advance UID and namespace leases beyond the
highest restored values, helping ensure subsequent assignments do not
reuse IDs already present in restored data.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
![https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3.svg"><source](https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3.svg"><source){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v3.svg"><img](https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v3.svg"><img){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source](https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source){kind=link}
![https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img](https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img){kind=link}
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.
Description
This PR bumps trunk's Go runtime to
go@1.27.3and golangci-lint to2.15.0, sotrunk checkcan lint the module again.#9864 moved
go.modtogo 1.27.3, but trunk still pinsgo@1.27.0and golangci-lint2.13.1. CI switches to the 1.27.3 toolchain automatically, and 1.27.3 writes export data version 5. Thex/toolsv0.49.0 importer inside golangci-lint 2.13.1 only reads up to version 4:So package
xfails to load, every promotedx.SafeMutexmethod reports as undefined, and trunk flags whichever of those land in a changed hunk. #9747 hit it first:#9864 itself passed because it changed no Go code. Any PR that touches Go files using
SafeMutex(or anything else inx) will fail the same way until this lands.golangci-lint 2.15.0 is built on
x/toolsv0.52.0 and reads the new export data. I reproduced the failure locally with 2.13.1, then ran 2.15.0 under Go 1.27.3 with the repo config: no new issues on #9747 under the default,integrationandintegration2build tags.Checklist
Conventional Commits syntax, leading
with
fix:,feat:,chore:,ci:, etc.Need help on this PR? Tag
@codesmith-botwith what you need.Summary by CodeRabbit