facebook / mysql-5.6

Facebook's branch of the Oracle MySQL database. This includes MyRocks.
http://myrocks.io
Other
2.46k stars 711 forks source link

Cleanup rdb_hexdump #1442

Closed laurynas-biveinis closed 3 months ago

laurynas-biveinis commented 3 months ago

Currently, rdb_hexdump has a third argument maxsize with a default value of 0, meaning unlimited output length, and there is also a RDB_MAX_HEXDUMP constant for the callers to use. However, only a part of the callers use it, the rest use the default or something else altogether (i.e. FN_REFLEN)

Change as follows:

Minor cleanups:

facebook-github-bot commented 3 months ago

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

luqun commented 3 months ago

This way it can dump any MyRocks key (3072 byte maximum).

This sounds incorrect. max_supported_key_part_length is 3072. max_supported_key_length is 16 * 1024 = 16k.

luqun commented 3 months ago

in Rdb_deadlock_info::Rdb_dl_trx_info get_dl_txn_info(), would you prefer calling rdb_hexdump with 0?

laurynas-biveinis commented 3 months ago

This sounds incorrect. max_supported_key_part_length is 3072. max_supported_key_length is 16 * 1024 = 16k.

Right, I missed this. Is it OK to bump the rdb_hexdump default to cover 16K keys?

laurynas-biveinis commented 3 months ago

in Rdb_deadlock_info::Rdb_dl_trx_info get_dl_txn_info(), would you prefer calling rdb_hexdump with 0?

Yes, updating

laurynas-biveinis commented 3 months ago

Rebased, addressed both comments (the rdb_hexdump one speculatively by bumping the default to 32K). Let me know if you prefer another default.

facebook-github-bot commented 3 months ago

@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.

laurynas-biveinis commented 3 months ago

+// The default limit for rdb_hexdump output length, happens to fit the longest +// possible keys (16K) +constexpr size_t RDB_MAX_HEXDUMP_LEN = 2 16 1024;

The comment mention 16K, but the value is 32K?

hexdump: two output bytes per one input byte

facebook-github-bot commented 3 months ago

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 3 months ago

This pull request has been merged in facebook/mysql-5.6@40f4f32475d1ff16f5315dfba44aa31aa53e29ee.