wrap memdb methods to return `Copy` instead of `any`
- Dominant language
- Go
- Stars
- 17k
- Forks
- 2.1k
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 105
Description
The Nomad [state store architecture guide](https://github.com/hashicorp/nomad/blob/main/contributing/architecture-state-store.md) describes an important constraint on state store methods:
> Every object you read from the state store must be copied before it can be mutated, because mutating the object modifies it outside the raft workflow. The result can be servers having inconsistent state, transactions breaking, or even server panics.
Unfortunately this is hard to statically enforce reliably except over "short distances" in the code. This kind of problem is called "taint analysis". We've done hack projects like https://github.com/hashicorp/nomad/pull/16257 to try to detect these bugs, but the results are currently still very buggy. So we're forced to rely on thorough code review, and even that sometimes fails and we get bugs like https://github.com/hashicorp/nomad/pull/23673
This bug has two parts:
1. The struct was missing the `Copy` method
2. And therefore we didn't call the `Copy` method
@Juanadelacuesta has suggested in an internal discussion that we could wrap state store methods such that they return `Copy` interface (`Copier`? `Copyable`?) rather than `any`. That would at least prevent problem 1.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with contributing/architecture-state-store.md, then inspect the state-store methods and Copy-related types involved in the bug described in #23673. Done means agreeing on and implementing a wrapper or interface design that makes those methods return a Copy-compatible value rather than any.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- distributed-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100