List:Internals« Previous MessageNext Message »
From:Jay Pipes Date:May 12 2009 4:39pm
Subject:Re: MySQL Reengineering Project
View as plain text  
Jay Pipes wrote:
> Mats Kindahl wrote:
>>
>> Alaric Snell-Pym wrote:
>>> Excellent news!
>>>
>>> One word of warning, though: make sure it's a series of small steps.
>>> It's far too easy, with this sort of thing, ending up going off on
>>> huge yak-shaving tangents. By all means take lots of small steps
>>> towards a lofty distant goal, but make sure each step is useful in its
>>> own right (even if just by allowing other steps to happen), or you can
>>> get lost on a branch that will never merge ;-)
>>
>> Yes, we don't want to do the work in macro steps (at least not at this 
>> point),
>> we want to proceed carefully.
>>
>>> I see that a few macroscopic tasks have appeared on the Forge already,
>>> but I'd like to add something I think could be changed for the better,
>>> on a grass-roots level throughout the codebase:
>>>
>>> I see a lot of methods that are called with arguments and return a
>>> value, but most of their input and output is actually through member
>>> fields of the object - not that the method is operating on its object
>>> per se, but that the caller actually puts things into the member
>>> fields, then calls the method, then inspects the results in member
>>> fields. For example, in the storage engine API, update_row is called
>>> with a buffer in unireg format, which it almost universally ignores
>>> and instead uses the array of Field objects set up in the handler
>>> object by the caller. And we spent some time in debugging our storage
>>> engine - it would return rows fine when you did selects on the table,
>>> but when you did certain types of join, it would fail to return any
>>> rows, despite our logging clearly showing we'd returned rows to MySQL
>>> - because it seems that sometimes MySQL not only looks at the return
>>> value of rnd_next, but also checks the 'status' member of the table
>>> object to see if the current row of the table is valid or not. So our
>>> rnd_next had to assign success/failure to table->status as well as
>>> returning success, and then everything worked OK. Doh.
>>>
>>> Making the inputs and outputs of every method/function explicit,
>>> rather than sneaking stuff in and out via members, will make the
>>> calling interfaces between things a lot easier to read, which will
>>> reduce the chances of developers working on a module introducing bugs!
>>> Plus, it'll simplify the classes a lot, and make them easier to read,
>>> as they will end up with only members that really relate to the actual
>>> domain object - eg, the table or whatever - rather than members that
>>> are part of the calling protocol of particular operations on the
>>> class. Less short-term mutable state in classes means they can be
>>> shared between threads in more and more contexts, too, as function
>>> arguments and return values live only on the thread-local stacks!
>>
>> Yes; the fact that the handler interface doesn't really honor the 
>> arguments has
>> been a major bummer for me several times. This is actually because 
>> some engines
>> that we support internally ignore the argument and use the stored records
>> record[0] and record[1] instead, which means that every engine (with the
>> exception of a few) started doing that. So now you have to both pass the
>> argument and the record to make sure that all engines work.
>>
>> Just getting clear semantics on how this part of the handler interface 
>> works,
>> and add assertion to weed out the bad usages, would simplify the code
>> significantly and improve the speed for all.
> 
> ++
> 
>> However, do you know of any other interfaces that work this way? I am 
>> personally
>> not aware of any other, but then I don't know every corner of the code 
>> like Serg
>> does. :)
> 
> "External" interfaces?  See all the plugin "interfaces".  There's no 
> enforcement of types really at all. Just passing void *'s around.
> 
> As for the internal interfaces, I would suggest cleaning up the class 
> interfaces of THD, JOIN, and other major classes to enforce public 
> accessors and getters, protecting private member variables behind a 
> clean API.  This would, eventually, make some of these classes 
> semi-usable in public interfaces.  Right now, the passing of the THD* 
> everywhere, and THD having basically a bunch of public member variables, 
> means that there is no enforcement of state changes through an 
> interface.  This leads to serious problems where the "internal" state of 
> a THD is actually public and cannot be seen as reliable for the lifetime 
> of a session's requests.
> 
> Another thing to think about in your refactoring efforts is detaching 
> the THD from its current inheritance from Statement, Query_arena and 
> ilink.  Without doing this, and using encapsulation so that a THD can 

s/encapsulation/composition/

> have multiple Statements, it will be very difficult to work on any 
> future parallelization efforts.
> 
> Cheers,
> 
> Jay
> 

Thread
MySQL Reengineering ProjectManyi Lu - Sun Norway12 May
  • Re: MySQL Reengineering ProjectJay Pipes12 May
  • Re: MySQL Reengineering ProjectMARK CALLAGHAN12 May
    • Re: MySQL Reengineering ProjectMats Kindahl12 May
      • Re: MySQL Reengineering ProjectManyi Lu - Sun Norway12 May
        • Re: MySQL Reengineering ProjectMats Kindahl12 May
    • Re: MySQL Reengineering ProjectStewart Smith13 May
  • Re: MySQL Reengineering ProjectMikiya Okuno18 May
    • Re: MySQL Reengineering ProjectSergei Golubchik18 May
      • Re: MySQL Reengineering ProjectRoy Lyseng18 May
Re: MySQL Reengineering ProjectAlaric Snell-Pym12 May
  • Re: MySQL Reengineering ProjectMats Kindahl12 May
    • Re: MySQL Reengineering ProjectJay Pipes12 May
      • Re: MySQL Reengineering ProjectJay Pipes12 May
        • Re: MySQL Reengineering ProjectAlex Esterkin12 May
          • Re: MySQL Reengineering ProjectBrian Aker12 May
      • Re: MySQL Reengineering ProjectSergei Golubchik12 May
        • Re: MySQL Reengineering ProjectBrian Aker12 May
          • Re: MySQL Reengineering ProjectSergei Golubchik12 May
            • Re: MySQL Reengineering ProjectAlex Esterkin12 May
              • Re: MySQL Reengineering ProjectBrian Aker12 May
            • Re: MySQL Reengineering ProjectJay Pipes12 May
              • Re: MySQL Reengineering ProjectDavi Arnaut13 May
              • Re: MySQL Reengineering ProjectAlex Esterkin13 May
                • Re: MySQL Reengineering ProjectJay Pipes13 May
            • Re: MySQL Reengineering ProjectMichael Widenius14 May
          • Re: MySQL Reengineering ProjectJonas Oreland13 May
        • Re: MySQL Reengineering ProjectRoy Lyseng12 May
      • Re: MySQL Reengineering ProjectMats Kindahl12 May
    • Re: MySQL Reengineering ProjectAlaric Snell-Pym12 May
      • Re: MySQL Reengineering ProjectMats Kindahl12 May
      • Re: MySQL Reengineering ProjectMichael Widenius14 May
        • Re: MySQL Reengineering ProjectAlaric Snell-Pym15 May
          • Re: MySQL Reengineering ProjectKristian Nielsen15 May
            • Re: MySQL Reengineering ProjectBrian Aker15 May
            • Re: MySQL Reengineering ProjectAlaric Snell-Pym15 May
          • Re: MySQL Reengineering ProjectMichael Widenius6 Jun
Re: MySQL Reengineering ProjectMats Kindahl12 May
Re: MySQL Reengineering ProjectAlex Esterkin12 May
  • Re: MySQL Reengineering ProjectMasood Mortazavi13 May
    • Re: MySQL Reengineering ProjectBrian Aker13 May
    • Re: MySQL Reengineering ProjectAlex Esterkin13 May
      • Re: MySQL Reengineering ProjectBaron Schwartz13 May
        • Re: MySQL Reengineering ProjectStewart Smith13 May
          • Re: MySQL Reengineering ProjectBaron Schwartz13 May
    • Re: MySQL Reengineering ProjectAlex Esterkin13 May
    • Re: MySQL Reengineering ProjectStewart Smith13 May
  • Re: MySQL Reengineering ProjectMichael Widenius14 May