Code review comment for lp:~stewart/percona-server/5.6-doublewrite

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

I have logged bugs for the review points that apply to 5.1 and 5.5 too.

I think that for 5.6 everything except for bug 1078181 must be fixed at the MP.

- The main issue with this feature in general is bug 1078181.
- The testcase must do some actual work with data pages. For
  example, the workload must be larger than the log capacity. (bug
  1078187).
- The sys_vars testcase should be a bit more involved. (bug 1078188).
- trx_sys_doublewrite_space() should be used everywhere it
  applies. (bug 1078191).
- buf_dblwr_get space_id arg should have comment.
- buf_dblwr_create_low header should follow InnoDB coding style.
- s/SRV_LOG_SPACE_FIRST_ID/SRV_EXTRA_SYS_SPACE_FIRST_ID in several
  places (bug 1078201).
- Chunk at 410--411 looks wrong: fil_is_user_tablespace_id looks like
  a correct check already.
- Bogus UNIV_LIKELY/UNIV_UNLIKELY (bug 1078206).
- trx_sys_dummy_create() is missing comments and misformatted in
  trx0sys.h.
- For initalizations like at 255--261 and 296--302 personally I prefer
  the ternary operator.
- trx_sys_sys_space has wrong header comment. "Check if a space is a
  system tablespace", likewise for trx_sys_doublewrite_space.
- Style violation at line 766.
- Too long lines at 769+, 780, 822, 825+, 857, 874
- Spurious diff at lines 428, 436, 495, 507, 576.

review: Needs Fixing

« Back to merge proposal