← Back to team overview

maria-developers team mailing list archive

MDEV-6089 review

 

Hi.

Here're I'll be asking questions about the patches I'm looking at.

1. Is this mostly a merge of WL#7305 or mostly your own implementation?
   That is, most of the changes and design decisions are yours or
   "because MySQL did it"?

2. Do you work like I do (commit, push, fix failing tests, combine
   commits with "git rebase -i" to get a clean history) or you've just
   magically got 32bit rdiffs correctly in every commit? :)

3. Why "MDL objects cache won't be needed"? The comment said "On some
   systems (e.g. Windows XP) constructing/destructing MDL_lock objects
   can be fairly expensive. We use this cache to avoid these costs..."
   Why is this no longer an issue?

4. MDL_lock_strategy - this object is in 5.7 too, but you've done it
   very differently. Why? And I wonder why didn't you move the common
   part of two strategies to the base class. You wanted a pure interface
   class?

5. Added initializer callback to lf-hash. May be you should remove this
   hack with decreasing LF_HASH::element_size etc and move this
   functionality to a custom initializer? It was added in 6ba12f.
   In particular, see changes to lf* files and whether they can use your
   initializer instead.

6. Replaced hash with lock-free hash: test result changes. Is the order
   of rows stable? perhaps you should use --sort_results ?

   in MDL_lock constructor, I'd set m_strategy to 0 (ori to 1 or, may
   be, keep it uninitialized) as a safety measure, to ensure it's not
   used before MDL_lock::lf_hash_initializer().

Otherwise looks ok. Mostly a straightforward change.

Regards,
Sergei



Follow ups