List:Internals« Previous MessageNext Message »
From:Jay Pipes Date:May 12 2009 3:28pm
Subject:Re: MySQL Reengineering Project
View as plain text  
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 
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