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

mirror: non-deterministic behavior if go mod list produces two packages with the same path #143168

Closed
rickystewart opened this issue Mar 19, 2025 · 3 comments · Fixed by #143169
Closed
Labels
A-build-system branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-25.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-dev-inf target-release-23.2.23 target-release-24.1.16 target-release-24.3.10 target-release-25.1.4 target-release-25.2.0

Comments

@rickystewart
Copy link
Collaborator

rickystewart commented Mar 19, 2025

Today, if you do go mod download -json, you see the following:

{
	"Path": "github.com/axiomhq/hyperloglog",
	"Version": "v0.2.0",
	"Info": "/Users/ricky/go/pkg/mod/cache/download/github.com/axiomhq/hyperloglog/@v/v0.2.0.info",
	"GoMod": "/Users/ricky/go/pkg/mod/cache/download/github.com/axiomhq/hyperloglog/@v/v0.2.0.mod",
	"Zip": "/Users/ricky/go/pkg/mod/cache/download/github.com/axiomhq/hyperloglog/@v/v0.2.0.zip",
	"Dir": "/Users/ricky/go/pkg/mod/github.com/axiomhq/[email protected]",
	"Sum": "h1:u1XT3yyY1rjzlWuP6NQIrV4bRYHOaqZaovqjcBEvZJo=",
	"GoModSum": "h1:GcgMjz9gaDKZ3G0UMS6Fq/VkZ4l7uGgcJyxA7M+omIM="
}
{
	"Path": "github.com/axiomhq/hyperloglog",
	"Version": "v0.0.0-20181223111420-4b99d0c2c99e",
	"Info": "/Users/ricky/go/pkg/mod/cache/download/github.com/axiomhq/hyperloglog/@v/v0.0.0-20181223111420-4b99d0c2c99e.info",
	"GoMod": "/Users/ricky/go/pkg/mod/cache/download/github.com/axiomhq/hyperloglog/@v/v0.0.0-20181223111420-4b99d0c2c99e.mod",
	"Zip": "/Users/ricky/go/pkg/mod/cache/download/github.com/axiomhq/hyperloglog/@v/v0.0.0-20181223111420-4b99d0c2c99e.zip",
	"Dir": "/Users/ricky/go/pkg/mod/github.com/axiomhq/[email protected]",
	"Sum": "h1:190ugM9MsyFauTkR/UqcHG/mn5nmFe6SvHJqEHIrtrA=",
	"GoModSum": "h1:IOXAcuKIFq/mDyuQ4wyJuJ79XLMsmLM+5RdQ+vWrL7o="
}

i.e., there are two packages with the same path. The mirroring code assumes this will not be the case and treats it as a unique key. This is typically a sensible assumption but no longer holds since we are using replace to effectively import two different versions of the same dependency.

Unfortunately this has resulted in non-determinism as the code may randomly select the "wrong" hyperloglog. This has resulted in a version of this library being uploaded to the bucket that was incorrect (it's labeled as being version 0.2.5, when it was actually v0.0.0-20181223111420-4b99d0c2c99e).

Jira issue: CRDB-48687

@rickystewart rickystewart added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Mar 19, 2025
Copy link

blathers-crl bot commented Mar 19, 2025

Hi @rickystewart, please add branch-* labels to identify which branch(es) this C-bug affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

rickystewart added a commit to rickystewart/cockroach that referenced this issue Mar 19, 2025
This code previously assumed that `go mod download -json` would not
produce two different versions of the same dependency with the same
path. This is typically a sensible assumption but no longer holds in
some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod`
to effectively import two different versions of the same dependency
with the same path. This results in non-determinism in the mirroring
code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We
also add some additional validation to check assumptions so if these
assumptions are ever broken in the future, the tool will fail loudly
instead of proceeding silently and performing a potentially harmful
operation.

Fixes cockroachdb#143168
Epic: CRDB-17171

Release note: None
rickystewart added a commit to rickystewart/cockroach that referenced this issue Mar 19, 2025
This code previously assumed that `go mod download -json` would not
produce two different versions of the same dependency with the same
path. This is typically a sensible assumption but no longer holds in
some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod`
to effectively import two different versions of the same dependency
with the same path. This results in non-determinism in the mirroring
code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We
also add some additional validation to check assumptions so if these
assumptions are ever broken in the future, the tool will fail loudly
instead of proceeding silently and performing a potentially harmful
operation.

Fixes cockroachdb#143168
Epic: CRDB-17171

Release note: None
craig bot pushed a commit that referenced this issue Mar 20, 2025
143100: kv: use origin timestamp to validate lww semantics r=jeffswenson a=jeffswenson

The PR updates the implementation of put, cput, delete, and range delete
to return an error if a replicated write with an origin timestamp would
overwrite a value with a more recent origin or MVCC timestamp.

No explicit version checks are included. In a mixed-version environment,
a SQL writer running 25.1 and writing to a 25.2 KV server will encounter
"condition failed" errors if an insert would overwrite a tombstone and
retry the write until it is upgraded or the LDR PTS ages out. This is
acceptable, as LDR remains in preview and the SQL writer is neither the
default nor the recommended writer.

Release note: none
Epic: [CRDB-48647](https://cockroachlabs.atlassian.net/browse/CRDB-48647)

143169: mirror: fix non-determinism in case two packages have the same path r=rail a=rickystewart

This code previously assumed that `go mod download -json` would not produce two different versions of the same dependency with the same path. This is typically a sensible assumption but no longer holds in some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod` to effectively import two different versions of the same dependency with the same path. This results in non-determinism in the mirroring code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We also add some additional validation to check assumptions so if these assumptions are ever broken in the future, the tool will fail loudly instead of proceeding silently and performing a potentially harmful operation.

Fixes #143168
Epic: CRDB-17171

Release note: None

Co-authored-by: Jeff Swenson <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
craig bot pushed a commit that referenced this issue Mar 20, 2025
143169: mirror: fix non-determinism in case two packages have the same path r=rail a=rickystewart

This code previously assumed that `go mod download -json` would not produce two different versions of the same dependency with the same path. This is typically a sensible assumption but no longer holds in some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod` to effectively import two different versions of the same dependency with the same path. This results in non-determinism in the mirroring code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We also add some additional validation to check assumptions so if these assumptions are ever broken in the future, the tool will fail loudly instead of proceeding silently and performing a potentially harmful operation.

Fixes #143168
Epic: CRDB-17171

Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
@craig craig bot closed this as completed in 87769f7 Mar 20, 2025
Copy link

blathers-crl bot commented Mar 20, 2025

Please add branch-* labels to identify which branch(es) this C-bug affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link

blathers-crl bot commented Mar 20, 2025

Based on the specified backports for linked PR #143169, I applied the following new label(s) to this issue: branch-release-24.1, branch-release-24.3, branch-release-25.1. Please adjust the labels as needed to match the branches actually affected by this issue, including adding any known older branches.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-25.1 labels Mar 20, 2025
rickystewart added a commit to rickystewart/cockroach that referenced this issue Mar 20, 2025
This code previously assumed that `go mod download -json` would not
produce two different versions of the same dependency with the same
path. This is typically a sensible assumption but no longer holds in
some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod`
to effectively import two different versions of the same dependency
with the same path. This results in non-determinism in the mirroring
code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We
also add some additional validation to check assumptions so if these
assumptions are ever broken in the future, the tool will fail loudly
instead of proceeding silently and performing a potentially harmful
operation.

Fixes cockroachdb#143168
Epic: CRDB-17171

Release note: None
rickystewart added a commit to rickystewart/cockroach that referenced this issue Mar 20, 2025
This code previously assumed that `go mod download -json` would not
produce two different versions of the same dependency with the same
path. This is typically a sensible assumption but no longer holds in
some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod`
to effectively import two different versions of the same dependency
with the same path. This results in non-determinism in the mirroring
code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We
also add some additional validation to check assumptions so if these
assumptions are ever broken in the future, the tool will fail loudly
instead of proceeding silently and performing a potentially harmful
operation.

Fixes cockroachdb#143168
Epic: CRDB-17171

Release note: None
rickystewart added a commit to rickystewart/cockroach that referenced this issue Mar 20, 2025
This code previously assumed that `go mod download -json` would not
produce two different versions of the same dependency with the same
path. This is typically a sensible assumption but no longer holds in
some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod`
to effectively import two different versions of the same dependency
with the same path. This results in non-determinism in the mirroring
code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We
also add some additional validation to check assumptions so if these
assumptions are ever broken in the future, the tool will fail loudly
instead of proceeding silently and performing a potentially harmful
operation.

Fixes cockroachdb#143168
Epic: CRDB-17171

Release note: None
rickystewart added a commit to rickystewart/cockroach that referenced this issue Mar 20, 2025
This code previously assumed that `go mod download -json` would not
produce two different versions of the same dependency with the same
path. This is typically a sensible assumption but no longer holds in
some niche scenarios. We use [replace](https://github.com/cockroachdb/cockroach/blob/65b2ed4fbdf5502f3fbe0af4ddbd30a7ac7eabb4/go.mod#L500) in `go.mod`
to effectively import two different versions of the same dependency
with the same path. This results in non-determinism in the mirroring
code with respect to which version of the dependency we select.

We now disambiguate with a path/version pair, which will be unique. We
also add some additional validation to check assumptions so if these
assumptions are ever broken in the future, the tool will fail loudly
instead of proceeding silently and performing a potentially harmful
operation.

Fixes cockroachdb#143168
Epic: CRDB-17171

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-system branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 branch-release-25.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-dev-inf target-release-23.2.23 target-release-24.1.16 target-release-24.3.10 target-release-25.1.4 target-release-25.2.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant