List:Commits« Previous MessageNext Message »
From:Vladislav Vaintroub Date:June 20 2008 6:45pm
Subject:RE: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172
View as plain text  
Hi Olav,

>    -In SerialLog::createNewWindow you have an optimization to only read
> the size of the file if the highWater == 0. This saves a system call
> once i n a while, but if I understand the code correctly this code only
> runs each time you actually switches to a new log file. If this is the
> case, I am not sure if an extra system call is worth to optimize away?

I'm not sure myself, actually the switch happens probably on every written
1MB, 
if transactions are  short. Yes, maybe it is not worth optimizing given
we've just written (unbuffered) 1MB or more.
I agree it would look simpler without hijacking highWater and this is kind
of premature optimization
(If a real measurement says we're doing to many system calls, we can get
this back).
Ok, I'll remove that part.

> 
>    -For the two new member functions in the SerialLogFile class you use
> uint64 for representing the size of the file. The exiting member
> functions and internal variables use int64 for representing the file
> size.

I originally planned to argue about that one. Neither file size nor file
offset can be negative.
off_t is signed probably  only to indicate error in ftell, while we're
indicating error by throwing exceptions.
And I wanted to show a good file IO API to prove my point. And here I
failed. 
Both java and C# believe in negative sizes and declare java.io.File.length
() as long rsp  System.IO.FileInfo.Length as int64.
Whatever, I will not try to outsmart java and C#, maybe it is good for a
newly envisioned "blackhole" filesystem with negative file sizes 
in the future. 
Thus agree to change to int64 ;)

> 
>   -There seems to be something wrong with the formatting of the last
> line in SerialLogFile::truncate. It seems like "highWater=size" have
> some extra indention (at least in my email reader).

Yes.

>   -In ha_falcon.cpp it seems like you introduce an un-necessary blank
> line after the ulonglong falcon_serial_log_file_size definition.

Will check.

>   -In the explaining text for the new parameter (in ha_falcon.cpp) I
> think maybe we should add the word "serial" to the text since we
> normally call it "the serial log", eg:
> 
>      "If *SERIAL* log file grows larger than this....."

OK.

Thanks you for review,
Vlad.


> -----Original Message-----
> From: Olav.Sandstaa@stripped [mailto:Olav.Sandstaa@stripped]
> Sent: Friday, June 20, 2008 1:52 PM
> To: Vladislav Vaintroub
> Cc: commits@stripped
> Subject: Re: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712)
> WL#4172


Thread
bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172Vladislav Vaintroub19 Jun
  • Re: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172Olav Sandstaa20 Jun
    • RE: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172Vladislav Vaintroub20 Jun
    • RE: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172Kevin Lewis22 Jun
      • RE: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172Vladislav Vaintroub22 Jun
        • RE: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172Kevin Lewis23 Jun
        • Re: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712)WL#4172Sergei Golubchik23 Jun
        • Re: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712) WL#4172Olav Sandstaa24 Jun