-
Notifications
You must be signed in to change notification settings - Fork 72
support stale-read testcase #416
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: yisaer <[email protected]> add stale-read test Signed-off-by: yisaer <[email protected]> add stale-read test Signed-off-by: yisaer <[email protected]>
e9d1016
to
ccb97e0
Compare
Signed-off-by: yisaer <[email protected]>
Signed-off-by: yisaer <[email protected]>
Signed-off-by: yisaer <[email protected]> fix lint Signed-off-by: yisaer <[email protected]>
4a73dd9
to
a88ccf0
Compare
testcase/stale-read/bench.go
Outdated
setSQL := fmt.Sprintf(`SET TRANSACTION READ ONLY as of timestamp tidb_bounded_staleness('%v', '%v')`, previousStr, nowStr) | ||
_, err := db.Exec(setSQL) | ||
if err != nil { | ||
return err | ||
} | ||
rows, err := db.Query("select id, k, c, pad from sbtest0 where k in (?, ?, ?)", rand.Intn(num), rand.Intn(num), rand.Intn(num)) | ||
defer rows.Close() | ||
if err != nil { | ||
return errors.WithStack(err) | ||
} | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to do COMMIT
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need to commit here.
testcase/stale-read/bench.go
Outdated
return nil | ||
} | ||
|
||
func (c *SysbenchCase) executeSelect(db *sql.DB) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a prepare test case is needed after pingcap/tidb#25156 is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can add it after tidb#25156 merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding a TODO.
testcase/stale-read/bench.go
Outdated
func (c *SysbenchCase) executeSelect(db *sql.DB) error { | ||
num := c.insertCount * c.rowsEachInsert | ||
now := time.Now() | ||
previous := now.Add(-3 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about making the staleness second 3s
configurable as a cmd flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated.
} | ||
|
||
// TODO: assert store read bytes/keys metrics | ||
func handleStoreMetricsValue(smvms map[string][]StoreMetricsValue, duration time.Duration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are you going to do with these metrics data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asserting the key/byte loads of store is close with each other.
Signed-off-by: yisaer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM.
testcase/stale-read/bench.go
Outdated
return nil | ||
} | ||
|
||
func (c *SysbenchCase) executeSelect(db *sql.DB) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding a TODO.
What problem does this PR solve?
What is changed and how does it work?
Check List
Add stale-read testcases.
Does this PR introduce a user-facing change?: