WIP - meta: add sequence generator; add SET -> DEL -> SINGLEDEL sequence #1254
Conversation
Currently, sequences of operations for metamorphic tests are generated for certain specific cases (e.g. a SINGLEDEL follows a key that has been SET once). Additional test sequences require maintaining the "state" of the sequence in the `generator`, which clutters the struct with various fields required for the state management. Add a `sequenceGenerator` struct, which uses a "transition map" (a mapping from a current state to next state, along with a corresponding output for the transition) to model a state machine. The state machine can be used to generate random sequences of operations that adhere to certain rules (e.g. output a GET following a SET for a given key). A `generator` can be constructed containing one or more `sequenceGenerator`s that, when selected by the random number generator (i.e. the "deck"), generate the next operation in the sequence governed by the state machine and place the operation into the operation log. Related to cockroachdb/cockroach#69414.
Add a `sequenceGenerator` instance with a transition map that generates the following sequence of operations, similar to the problematic sequence generated for cockroachdb/cockroach#69414: ``` ((SET -> GET)+ -> DELETE -> GET)+ -> SINGLEDEL -> (GET)+ ``` See also cockroachdb/cockroach#69891.
|
This change is |
|
Marking this as WIP, as we probably don't want to merge the second commit until we work out whether this needs to be fixed at the pebble layer (in which case we'll want the fix to accompany the addition of the sequence). |
There was a problem hiding this comment.
Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @sumeerbhola)
internal/metamorphic/scenario.go, line 18 at r2 (raw file):
// fetching the key after it has been single-deleted. // // This sequenceGenerator is regression test for cockroachdb/cockroach#69414.
At this SHA, we can force a metamorphic test failure with seed 1631054105813493000.
There was a problem hiding this comment.
Reading cockroachdb/cockroach#69891, we can probably save the second commit here for when the final item in that issue is merged into pebble, which will serve as a regression test.
Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @sumeerbhola)
There was a problem hiding this comment.
Reviewed 3 of 4 files at r1, 1 of 3 files at r2.
Reviewable status: 4 of 6 files reviewed, 3 unresolved discussions (waiting on @nicktrav)
internal/metamorphic/scenario.go, line 20 at r2 (raw file):
// This sequenceGenerator is regression test for cockroachdb/cockroach#69414. func makeDelToSingleDelSequence(key []byte, valueFn func() []byte) *sequenceGenerator { reader, writer := makeObjID(dbTag, 0), makeObjID(dbTag, 0)
I see why this state machine seems simpler than tracking key states.
- Should this be setup such that keys used in such sequence generators are never to be used for writes by the usual operations. They can of course be used for reads.
- This restricts us to performing operations on the DB directly since it avoids the complexity of tracking the key through various writers. Is there a way to make this more sophisticated without sacrificing the simplicity?
I would like to see a code attempt at tracking the state of keys in a central place, as an alternative, and see if the complexity of that is not too high (my comment in #1248)
internal/metamorphic/scenario.go, line 24 at r2 (raw file):
var statePrev opType var doneDel, doneSingleDel bool var setCount, trailingGetCount int
I wonder if the state management in this code would be easier to read if this were a struct implementing an interface and the interface was what was driven by the sequenceGenerator. There is clearly some shared state that these funcs need and it is being provided by the surrounding environment, which is less readable than if these were methods on a struct.
Currently, the metamorphic test generator tracks the state of keys that it generates to determine whether or not certain keys can be used for specific operations. Specifically, keys that have been set once in the database are eligible to be SingleDeleted. Future enhancements to the test suite will likely introduce other desirable sequences of operations that rely on tracking the state of certain keys through the metamorphic test. State tracking for keys is currently embedded directory onto the `generator` struct. Additional fields that are to be used for state tracking would continue to complicate the already sizable struct. There is also significant cognitive overhead with the existing state tracking for keys. Introduce the concept of a "key manager", dedicated to tracking the state of keys during metamorphic test generation time. Internally, the `keyManager` struct can be passed operations via `op`s` (e.g. `setOp`, etc.) that transform the internal state. The `keyManager` then exposes functions that can be used to deduce which keys are eligible for certain operations at a point in time. Initially, one such function is `eligibleSingleDeleteKeys`, which is used to replace the existing logic in the `generator` for tracking keys that can be SingleDeleted. Future enhancements to the metamorphic test suite that rely on state tracking can now make use of the dedicated state management responsibilities of the `keyManager`. Informed by cockroachdb#1248, cockroachdb#1254. Related to cockroachdb#1255.
Currently, the metamorphic test generator tracks the state of keys that it generates to determine whether or not certain keys can be used for specific operations. Specifically, keys that have been set once in the database are eligible to be SingleDeleted. Future enhancements to the test suite will likely introduce other desirable sequences of operations that rely on tracking the state of certain keys through the metamorphic test. State tracking for keys is currently embedded directory onto the `generator` struct. Additional fields that are to be used for state tracking would continue to complicate the already sizable struct. There is also significant cognitive overhead with the existing state tracking for keys. Introduce the concept of a "key manager", dedicated to tracking the state of keys during metamorphic test generation time. Internally, the `keyManager` struct can be passed operations via `op`s` (e.g. `setOp`, etc.) that transform the internal state. The `keyManager` then exposes functions that can be used to deduce which keys are eligible for certain operations at a point in time. Initially, one such function is `eligibleSingleDeleteKeys`, which is used to replace the existing logic in the `generator` for tracking keys that can be SingleDeleted. Future enhancements to the metamorphic test suite that rely on state tracking can now make use of the dedicated state management responsibilities of the `keyManager`. Informed by cockroachdb#1248, cockroachdb#1254. Related to cockroachdb#1255.
|
Closing this out, as we have the |
This first commit in this PR adds the concept of a "sequence generator" to produce random sequences governed by a state machine.
The second adds a sequence to reproduce the bug we were seeing in cockroachdb/cockroach#69414.