Hi, Sergey! On Mar 03, Sergey Vojtovich wrote: > On Tue, Mar 03, 2015 at 03:38:31PM +0100, Sergei Golubchik wrote: > > 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"? > Implementations differ. All design decisions are mine, like if I were to > implement this from scratch. There is nothing like "because MySQL did it".
Great. I only wanted to know whether I should bug you about design choices or study MySQL implementation and try to guess from there. > > 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. > You mean I should fix waiting threads (and probably table cache) to use > initializer function instead of decreasing LF_HASH::element_size? I don't > mind, but probably in a separate patch. Separate patch is ok. My old implementation in 6ba12f used a small change to LF_HASH that was sufficient for waiting threads code. But if I had a proper initializer in LF_HASH I'd probably have used it, instead of coming with hacks like decreasing LF_HASH::element_size. It is kind of unobvious, so I'd prefer to get rid of it. > > 6. Replaced hash with lock-free hash: test result changes. Is the order > > of rows stable? perhaps you should use --sort_results ? > Order of rows depends on hash implementation. Okay, that means it's stable. > > 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(). > I guess it's for one of intermediate patches. Last version does it. MDL_lock has two constructors. One sets m_strategy to 0, the other to &m_scoped_lock_strategy. > > Otherwise looks ok. Mostly a straightforward change. > Should I take it as ok to push? :) Yes, please. Regards, Sergei _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
