Vlad, I would like to add this to Olav's comments, and also commentto his
comments inline.
1) I think it helps to debug when the location of an error is embedded in
the text. Like this;
if (!SetFilePointerEx(handle, distance ,&oldPos,FILE_CURRENT))
- throw SQLError(IO_ERROR, " SetFilePointerEx failed with %d",
+ throw SQLError(IO_ERROR, " SerialLogFile::truncate;
SetFilePointerEx failed with %d",
GetLastError());
2) I think I suggested something like this on IRC, cannot remember...
static MYSQL_SYSVAR_ULONGLONG(serial_log_file_size,
falcon_serial_log_file_size,
PLUGIN_VAR_RQCMDARG,
- "If log file grows larger than this value, it will be truncated",
+ "If a serial log file grows larger than this value, it will be truncated
when it is reused",
NULL, NULL , LL(10)<<20, LL(1)<<20, (ulonglong) ~0, LL(1)<<20);
More comments below;
>-----Original Message-----
>From: Olav.Sandstaa@stripped [mailto:Olav.Sandstaa@stripped]
>Sent: Friday, June 20, 2008 6:52 AM
>To: Vladislav Vaintroub
>Cc: commits@stripped
>Subject: Re: bzr commit into mysql-6.0-falcon branch (vvaintroub:2712)
>WL#4172
>
>Vlad,
>
>I think the patch looks very good. I have only minor comments and no
>objections to this being pushed.
>
>Note that I have only looked at the Non-Windows code, the WIN32 code I
>have no knowledge about.
>
>Minor comments:
>
> -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 in 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?
Disagree. (highWater == 0) means that you are actually going to switch
files. If it is not zero, which is most of the time by far, you don't want
to do this stuff.
>
> -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.
Agree to use signed.
> -In SerialLogFile::truncate() using uint64 as the file size you call
>ftruncate casting this to off_t which I think is a signed type on most
>systems so you do not gain anything by using a unsigned type (at least
>on Unixes)
Agree to use signed.
> -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).
????
> -In ha_falcon.cpp it seems like you introduce an un-necessary blank
>line after the ulonglong falcon_serial_log_file_size definition.
Disagree. I like the blank line, As does Jim, who has prescribed this kind
of spacing often.
> -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....."
>
Agree, see above.
>These are all only minor comments, leave it to you to decide if you want
>to change anything.
>
>OK to push this patch.
>
>Thanks for adding this to Falcon!
>
>Regards,
>Olav
>
>
>Vladislav Vaintroub wrote:
>> #At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-6.0-falcon/
>>
>> 2712 Vladislav Vaintroub 2008-06-19
>> WL#4172 : Truncate Falcon Log Files
>> Introduce a new parameter falcon_serial_log_file_size for maximum
>size of
>> a serial log file before it gets truncated. During the log switch,
>check the
>> file size. Truncate, if file is too big.
>> modified:
>> mysql-test/suite/falcon/r/falcon_options.result
>> mysql-test/suite/falcon/r/falcon_options2.result
>> storage/falcon/SerialLog.cpp
>> storage/falcon/SerialLogFile.cpp
>> storage/falcon/SerialLogFile.h
>> storage/falcon/ha_falcon.cpp
>>
>> per-file comments:
>> mysql-test/suite/falcon/r/falcon_options.result
>> New parameter falcon_serial_log_file_size
>> mysql-test/suite/falcon/r/falcon_options2.result
>> New parameter falcon_serial_log_file_size
>> storage/falcon/SerialLog.cpp
>> Truncate log file if it became too big.
>> For optimization reasons rely on SerialLogFile::highWater to be the
>file size, if highWater!=0
>> storage/falcon/SerialLogFile.cpp
>> New functions truncate() to truncate the log file , and size() to
>retrieve its size
>> storage/falcon/SerialLogFile.h
>> New functions truncate() to truncate the log file , and size() to
>retrieve its size
>> storage/falcon/ha_falcon.cpp
>> New parameter falcon_serial_log_file_size
>> === modified file 'mysql-test/suite/falcon/r/falcon_options.result'
>> --- a/mysql-test/suite/falcon/r/falcon_options.result 2008-03-11
>16:15:47 +0000
>> +++ b/mysql-test/suite/falcon/r/falcon_options.result 2008-06-18
>22:40:46 +0000
>> @@ -23,6 +23,7 @@ falcon_scavenge_schedule 15,45 * * * * *
>> falcon_serial_log_block_size 0
>> falcon_serial_log_buffers 20
>> falcon_serial_log_dir
>> +falcon_serial_log_file_size 10485760
>> falcon_serial_log_priority 1
>> falcon_support_xa OFF
>> falcon_use_deferred_index_hash OFF
>> @@ -111,6 +112,7 @@ FALCON_SCAVENGE_SCHEDULE 15,45 * * * * *
>> FALCON_SERIAL_LOG_BLOCK_SIZE 0
>> FALCON_SERIAL_LOG_BUFFERS 20
>> FALCON_SERIAL_LOG_DIR
>> +FALCON_SERIAL_LOG_FILE_SIZE 10485760
>> FALCON_SERIAL_LOG_PRIORITY 1
>> FALCON_SUPPORT_XA OFF
>> FALCON_USE_DEFERRED_INDEX_HASH OFF
>>
>> === modified file 'mysql-test/suite/falcon/r/falcon_options2.result'
>> --- a/mysql-test/suite/falcon/r/falcon_options2.result 2008-03-11
>16:15:47 +0000
>> +++ b/mysql-test/suite/falcon/r/falcon_options2.result 2008-06-18
>22:40:46 +0000
>> @@ -24,6 +24,7 @@ FALCON_SCAVENGE_SCHEDULE 15,45 * * * * *
>> FALCON_SERIAL_LOG_BLOCK_SIZE 0
>> FALCON_SERIAL_LOG_BUFFERS 20
>> FALCON_SERIAL_LOG_DIR
>> +FALCON_SERIAL_LOG_FILE_SIZE 10485760
>> FALCON_SERIAL_LOG_PRIORITY 1
>> FALCON_SUPPORT_XA OFF
>> FALCON_USE_DEFERRED_INDEX_HASH OFF
>>
>> === modified file 'storage/falcon/SerialLog.cpp'
>> --- a/storage/falcon/SerialLog.cpp 2008-04-12 02:22:50 +0000
>> +++ b/storage/falcon/SerialLog.cpp 2008-06-18 22:40:46 +0000
>> @@ -53,6 +53,7 @@ static const char THIS_FILE[]=__FILE__;
>> static const int TRACE_PAGE = 0;
>>
>> extern uint falcon_gopher_threads;
>> +extern uint64 falcon_serial_log_file_size;
>>
>> //static const int windowBuffers = 10;
>> static bool debug;
>> @@ -597,8 +598,24 @@ void SerialLog::createNewWindow(void)
>> break;
>> }
>>
>> - if (fileOffset == 0 && Log::isActive(LogInfo))
>> - Log::log(LogInfo, "%d: Switching log files (%d used)\n",
>database->deltaTime, file->highWater);
>> + if (fileOffset == 0)
>> + {
>> + // Logfile switch, truncate file if required
>> +
>> + if (Log::isActive(LogInfo))
>> + Log::log(LogInfo, "%d: Switching log files (%d
used)\n",
>> + database->deltaTime,
file->highWater);
>> +
>> + uint64 fileSize;
>> +
>> + if (file->highWater == 0)
>> + fileSize = file->size();
>> + else
>> + fileSize = file->highWater;
>> +
>> + if(fileSize > falcon_serial_log_file_size)
>> + file->truncate(falcon_serial_log_file_size);
>> + }
>>
>> writeWindow->deactivateWindow();
>> writeWindow = allocWindow(file, fileOffset);
>>
>> === modified file 'storage/falcon/SerialLogFile.cpp'
>> --- a/storage/falcon/SerialLogFile.cpp 2008-05-09 20:14:24 +0000
>> +++ b/storage/falcon/SerialLogFile.cpp 2008-06-18 22:40:46 +0000
>> @@ -305,6 +305,59 @@ uint32 SerialLogFile::read(int64 positio
>> #endif
>> }
>>
>> +void SerialLogFile::truncate(uint64 size)
>> +{
>> +#ifdef _WIN32
>> + LARGE_INTEGER oldPos, distance;
>> + distance.QuadPart = 0;
>> +
>> + // Get current position in file
>> + if (!SetFilePointerEx(handle, distance ,&oldPos,FILE_CURRENT))
>> + throw SQLError(IO_ERROR, "SetFilePointerEx failed with %d",
>> + GetLastError());
>> +
>> + // Position to the new end of file , set EOF marker there
>> + distance.QuadPart = size;
>> + if (!SetFilePointerEx(handle, distance, 0, FILE_BEGIN))
>> + throw SQLError(IO_ERROR, "SetFilePointerEx failed with %d",
>> + GetLastError());
>> +
>> + if (!SetEndOfFile(handle))
>> + throw SQLError(IO_ERROR, "SetEndOfFile failed with %d",
>> + GetLastError());
>> +
>> +
>> + // Restore file pointer
>> + if (!SetFilePointerEx(handle, oldPos ,0,FILE_BEGIN))
>> + throw SQLError(IO_ERROR, "SetFilePointerEx failed with %d",
>> + GetLastError());
>> +#else
>> + if (ftruncate(handle, (off_t) size))
>> + throw SQLError(IO_ERROR, "ftruncate failed with %d",
>> + errno);
>> +#endif
>> + highWater = MIN(highWater,(int64)size);
>> + highWater = size;
>> +}
>> +
>> +uint64 SerialLogFile::size(void)
>> +{
>> +#ifdef _WIN32
>> + LARGE_INTEGER size;
>> +
>> + if (!GetFileSizeEx(handle, &size))
>> + throw SQLError(IO_ERROR, "GetFileSizeEx failed with %u",
>> + GetLastError());
>> +
>> + return size.QuadPart;
>> +#else
>> + struct stat buf;
>> + if (fstat(handle, &buf))
>> + throw SQLError(IO_ERROR, "stat failed with %d",
>> + errno);
>> + return (uint64) buf.st_size;
>> +#endif
>> +}
>>
>> void SerialLogFile::zap()
>> {
>>
>> === modified file 'storage/falcon/SerialLogFile.h'
>> --- a/storage/falcon/SerialLogFile.h 2008-03-11 16:15:47 +0000
>> +++ b/storage/falcon/SerialLogFile.h 2008-06-18 22:40:46 +0000
>> @@ -38,6 +38,8 @@ public:
>> void write(int64 position, uint32 length, const SerialLogBlock
*data);
>> void close();
>> void open (JString filename, bool creat);
>> + void truncate(uint64 size);
>> + uint64 size(void);
>> SerialLogFile(Database *db);
>> virtual ~SerialLogFile();
>>
>>
>> === modified file 'storage/falcon/ha_falcon.cpp'
>> --- a/storage/falcon/ha_falcon.cpp 2008-06-02 11:19:39 +0000
>> +++ b/storage/falcon/ha_falcon.cpp 2008-06-18 22:40:46 +0000
>> @@ -80,6 +80,8 @@ static StorageHandler *storageHandler;
>>
>> ulonglong falcon_record_memory_max;
>> ulonglong falcon_initial_allocation;
>> +ulonglong falcon_serial_log_file_size;
>> +
>> uint falcon_allocation_extent;
>> ulonglong falcon_page_cache_size;
>> char* falcon_serial_log_dir;
>> @@ -3507,6 +3509,11 @@ static MYSQL_SYSVAR_ULONGLONG(initial_al
>> "Initial allocation (in bytes) of falcon user tablespace.",
>> NULL, NULL, 0, 0, LL(4000000000), LL(1)<<20);
>>
>> +static MYSQL_SYSVAR_ULONGLONG(serial_log_file_size,
>falcon_serial_log_file_size,
>> + PLUGIN_VAR_RQCMDARG,
>> + "If log file grows larger than this value, it will be truncated",
>> + NULL, NULL , LL(10)<<20, LL(1)<<20, (ulonglong) ~0,
> LL(1)<<20);
>> +
>> /***
>> static MYSQL_SYSVAR_UINT(allocation_extent, falcon_allocation_extent,
>> PLUGIN_VAR_OPCMDARG | PLUGIN_VAR_READONLY,
>> @@ -3554,6 +3561,7 @@ static struct st_mysql_sys_var* falconVa
>> //MYSQL_SYSVAR(allocation_extent),
>> MYSQL_SYSVAR(page_cache_size),
>> MYSQL_SYSVAR(consistent_read),
>> + MYSQL_SYSVAR(serial_log_file_size),
>> NULL
>> };
>>
>>
>>
>>
>
>
>--
>MySQL Code Commits Mailing List
>For list archives: http://lists.mysql.com/commits
>To unsubscribe: http://lists.mysql.com/commits?unsub=1
