MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Chuck Bell Date:July 1 2008 11:46pm
Subject:RE: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364, Bug#35230, WL#4296
View as plain text  
Rafal,

Thanks for your very detailed review. I wasn't expecting such an effort and
I am amazed you took the time to go through it in such fine detail -- above
and beyond in my book (that's a compliment).

Just a few comments. I haven't had time to digest your entire treatise but I
felt we need to discuss some fundamentals nonetheless. 

> Backup kernel/server separation
> -------------------------------
I understand your approach here and I don't disagree at a fundamental level.
However, we had agreed earlier (I don't have it in writing as it were) that
we were going to go down the KISS path for the logging in this worklog and
that enhancements to the interface and usage would be separate tasks. The
reason is we wanted to satisfy the architects' concerns first with the basic
functionality before launching into a more detailed approach. 

If we launch with a complex design, I fear a scenario like what is occurring
with si_objects. If we change how we do logging but don't change the rest of
the server at the same time it may confuse people and create unnecessary
debate. Perhaps this is unfounded, but that's how I see it.

> The backupdir option
> --------------------
> 
> I don't like that you included implementation of that option 
> as part of WL#4296. 
> Creating a monster task which includes several loosely 
> related subtasks makes 
> implementing and reviewing the changes much harder. I think 
> "divide and conquer" 
> is the best strategy here.

I am sorry you don't like including this here, but it was specifically
requested by Peter. I needed to implement backupdir in order to get the logs
to work properly. However, I have since figured out a way to separate the
code so I am willing to work with you and make it a separate patch. I don't
think that part would be a problem. However, it does need to be included
with this work.

As for how it interacts with the backup code, I must again caution that we
need to approach this in increments. Let's not try to design the minutia in
the first pass. I think that mentality gets us into hot water too quickly
(admittedly the opposite is equally bad). Rather, I'd like to see us get it
working now so that later we have a baseline of functionality with which we
can base improvements (not to mention mature) rather than trying to hit the
final target on the first go. I think you and I will always agree to
disagree on this point.

> Server installation
> ------------------
> When new server is installed, the backup logging tables 
> should be created inside 
> mysql database. I don't see that your patch provides code for that.

I will look into that. 

I think we need to decide now if I am going to implement your suggestions
for redesign. Please think about this some more before saying 'no -- it must
be my way' because we are only so many people and this work is on the list
of things that need to be done now. My time is even more constricted this
month and as it stands I will be lucky to have time to look at the code next
week. It is unfortunate that we did not have more time to plan this work in
more detail, but thems the breaks as they say.

What I propose is to make a new worklog detailing your ideas and class
designs as well as other design elements you have highlighted. In this way,
I think we can meet the goal of getting this work done and yet still have a
purpose driven goal of getting it in the desired state. Otherwise, I
honestly can't see this work getting done in July or even August (check the
leave charts). I feel I must add that we do not really need this level of
separation at this point. While I will agree we need to be conscious of the
need to separate, I feel that the code I have written does not hinder us
from doing so in the future (as is evident in your example patches).

Therefore, I ask you to please reconsider your detailed design changes.

Note: Many of the items regarding memory and such are things that are
inherent in the code now. A lot of the code for the variables, backupdir,
etc. are literally translations of existing code. I don't say this as an
excuse or substitute for quality, rather I say this as way of explanation
for how/why some of the code is written. I see no problems in making most of
the improvements but I reserve the right to reject some based on precedence
and conformity.

Chuck

Thread
bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364, Bug#35230,WL#4296Chuck Bell25 Jun
  • Re: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364,Bug#35230, WL#4296Rafal Somla30 Jun
    • RE: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364, Bug#35230, WL#4296Chuck Bell2 Jul
      • Re: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364,Bug#35230, WL#4296Rafal Somla2 Jul
        • Re: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364,Bug#35230, WL#4296Rafal Somla2 Jul
          • RE: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364, Bug#35230, WL#4296Chuck Bell2 Jul
        • RE: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364, Bug#35230, WL#4296Chuck Bell2 Jul
  • Re: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364,Bug#35230, WL#4296Øystein Grøvlen5 Aug
    • RE: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364, Bug#35230, WL#4296Chuck Bell5 Aug
      • Re: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364,Bug#35230, WL#4296Øystein Grøvlen7 Aug
        • RE: bzr commit into mysql-6.0-backup branch (cbell:2638) Bug#33364, Bug#35230, WL#4296Chuck Bell13 Aug