maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #08218
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