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 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?
-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.
-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)
-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.
-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....."
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
> };
>
>
>
>
