Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
68163: tpcc: fix error handling in check 3.3.2.4 r=erikgrinaker a=tbg

A classic footgun baked into the `database/sql` is that there's an
easy-to-miss error check:

```go
rows, err := db.Query(/* ... */)
if err != nil { return err }
for rows.Next() { /* stuff */ }
if rows.Err() != nil { // <------------- footgun
  return err
}
return rows.Close()
```

This was the case in our impl of TPCC check 3.3.2.4, which runs the
following queries (at whatever timestamp is chosen at the time):

```sql
SELECT sum(o_ol_cnt) FROM "order" AS OF SYSTEM TIME '2021-07-28 10:03:59.988263+00' GROUP BY o_w_id, o_d_id ORDER BY o_w_id, o_d_id;
SELECT count(*) FROM order_line   AS OF SYSTEM TIME '2021-07-28 10:03:59.988263+00' GROUP BY ol_w_id, ol_d_id ORDER BY ol_w_id, ol_d_id;
```

As of a recent change, the second query runs into the SQL memory budget
in our nightly tests. The missing error check was making that look like
a scary anomaly.

The other checks all have the same problem. I'll file an issue for that
and link it to this PR.

With this commit, the roachtest fails with

> Error: check failed: 3.3.2.4: on `order_line`: pq: root: memory budget
> exceeded: 10240 bytes requested, 3773291520 currently allocated,
> 3773295616 bytes in budget

as you'd expect.

Fixes cockroachdb#68151.

(though the memory limit will continue to fail the test until fixed
separately).

Release note: None


Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
  • Loading branch information
craig[bot] and tbg committed Jul 28, 2021
2 parents 412117e + a5936de commit 1159281
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion pkg/workload/tpcc/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,17 @@ ORDER BY
return errors.Errorf("order.sum(o_ol_cnt): %d != order_line.count(*): %d", left, right)
}
}
if err := leftRows.Err(); err != nil {
return errors.Wrap(err, "on `order`")
}
if err := rightRows.Err(); err != nil {
return errors.Wrap(err, "on `order_line`")
}
if i == 0 {
return errors.Errorf("0 rows returned")
}
if leftRows.Next() || rightRows.Next() {
return errors.Errorf("length of order.sum(o_ol_cnt) != order_line.count(*)")
return errors.Errorf("at %s: length of order.sum(o_ol_cnt) != order_line.count(*)", ts)
}

if err := leftRows.Close(); err != nil {
Expand Down

0 comments on commit 1159281

Please sign in to comment.