#At file:///G:/bzr/bug39138/
2808 Vladislav Vaintroub 2008-09-04
Bug#39138 : Falcon total database deletion after CREATE + DROP TABLESPACE + recovery
Problem: Falcon misinterpretins some errors from openDatabase(), assumes database does
not exist and tries to create a new database. During creation, it overwrites already existing
files (by passing O_CREAT|O_TRUNC to open() on Posix, and CREATE_ALWAYS to CreateFile on Windows).
This is corrected in this patch
- open() is now called with O_CREAT|O_EXCL and CreateFile() with CREATE_NEW.
This will make Falcon never overwrite existing files.
- To ensure the proper cleanup of files created during create database attempt,
"create" status is tracked. On error, createDatabase sets global deleteFilesOnExit
flag - this causes new files to be deleted in destructors of IO and SerialLogFile.
But there is no cleanup if process terminates with exception/signal.
A more elegant solution for cleanup is possible on file systems that support user-mode
transactions (like recent NTFS), by making the whole createDatabase/openDatabase a single
transaction with commit() at the end. Unfortunately this solution is not portable yet,
not even on downlevel Windows ,so it is not implemented.
- We also ensure with ASSERT, that createDatabase never opens existing files, but always
creates new ones.
- We do not attempt to create a database after failing recovery anymore.
modified:
storage/falcon/Database.cpp
storage/falcon/IO.cpp
storage/falcon/IOx.h
storage/falcon/SQLException.h
storage/falcon/SerialLogFile.cpp
storage/falcon/SerialLogFile.h
storage/falcon/StorageHandler.cpp
storage/falcon/StorageHandler.h
storage/falcon/ha_falcon.cpp
per-file messages:
storage/falcon/Database.cpp
-If createDatabase fails, set deleteFilesOnExit flag, instead of manually
deleting the master tablespace- there can be more files to delete than
just falcon_master.fts
- do not try opening serial log error in current directory if open fails
on some reason
-if recovery fails, throw new RECOVERY_ERROR exception. Good for diagnostics
storage/falcon/IO.cpp
- Correct open flags for createFile(). It must be O_CREAT|O_EXCL
not O_CREAT|O_TRUNC - in later case the file contents of existing file
is destroyed and this is not what we want to happen to database content.
Basically, never ever overwrite an existing file on createFile().
- Track "created" status for the files. It is used during cleanup
of the failing create database. If file was created and create
database fails, the file will be deleted during IO destructor.
- assert, that we do not open existing files on create database
(we should always create them)
storage/falcon/IOx.h
New variables
deleteFilesOnExit - indicates failed create database attempt
and used to cleanup created files.
inCreateDatabase - for assert that we never try to open already
existing files during "create database"
storage/falcon/SQLException.h
New error code RECOVERY_ERROR
storage/falcon/SerialLogFile.cpp
- Correct open flags for open with "create". It must be O_CREAT|O_EXCL not O_CREAT|O_TRUNC -
in later case the file contents of existing file is destroyed and this is not what we want.
Basically, never ever overwrite an existing file.
On Windows, use CREATE_NEW when creating a new file and OPEN_EXISTING when opening a file,
instead of CREATE_ALWAYS or OPEN_ALWAYS.
- Track "created" status for the files. It is used during cleanup
of the failing create database. If file was created and create
database fails, the file will be deleted during SerialLogFile
destructor.
storage/falcon/StorageHandler.cpp
-new function freeFalconStorageHandler
-instead of using 2 static variables storageHandler
in both StorageHandler.cpp and ha_falcon.cpp,
use single global variable.
- refactor code to create database from exception handler
in initialize() into dedicated function. On any error,
set global deleteFilesOnExit flags for proper cleanup.
storage/falcon/StorageHandler.h
new function createDatabase
storage/falcon/ha_falcon.cpp
- do a proper cleanup, if init() fails ( i.e call falcon_deinit())
when exception is caught
- when deinitializing falcon handler, free storageHandler memory
- simplify code to for shutdown()/panic() - as it does the same as deinit(),
just call deinit().
=== modified file 'storage/falcon/Database.cpp'
--- a/storage/falcon/Database.cpp 2008-08-11 13:22:53 +0000
+++ b/storage/falcon/Database.cpp 2008-09-04 10:49:57 +0000
@@ -679,11 +679,10 @@ void Database::createDatabase(const char
}
catch (...)
{
- dbb->closeFile();
- dbb->deleteFile();
-
+ deleteFilesOnExit = true;
throw;
}
+
}
void Database::openDatabase(const char * filename)
@@ -705,34 +704,18 @@ void Database::openDatabase(const char *
{
if (dbb->logLength)
serialLog->copyClone(dbb->logRoot, dbb->logOffset, dbb->logLength);
+ serialLog->open(dbb->logRoot, false);
+ if (dbb->tableSpaceSectionId)
+ tableSpaceManager->bootstrap(dbb->tableSpaceSectionId);
- try
+ try
{
- serialLog->open(dbb->logRoot, false);
+ serialLog->recover();
}
- catch (SQLException&)
+ catch(SQLError &e)
{
- const char *p = strrchr(filename, '.');
- JString logRoot = (p) ? JString(filename, (int) (p - filename)) : name;
- bool failed = true;
-
- try
- {
- serialLog->open(logRoot, false);
- failed = false;
- }
- catch (...)
- {
- }
-
- if (failed)
- throw;
+ throw SQLError(RECOVERY_ERROR, "Recovery failed: %s",e.getText());
}
-
- if (dbb->tableSpaceSectionId)
- tableSpaceManager->bootstrap(dbb->tableSpaceSectionId);
-
- serialLog->recover();
tableSpaceManager->postRecovery();
serialLog->start();
}
=== modified file 'storage/falcon/IO.cpp'
--- a/storage/falcon/IO.cpp 2008-08-29 20:41:13 +0000
+++ b/storage/falcon/IO.cpp 2008-09-04 10:49:57 +0000
@@ -114,6 +114,8 @@ static int simulateDiskFull = SIMULATE_D
static FILE *traceFile;
static char baseDir[PATH_MAX+1]={0};
+bool deleteFilesOnExit = false;
+bool inCreateDatabase = false;
#ifdef _DEBUG
#undef THIS_FILE
@@ -133,6 +135,7 @@ IO::IO()
dbb = NULL;
forceFsync = true;
fatalError = false;
+ created = false;
memset(writeTypes, 0, sizeof(writeTypes));
syncObject.setName("IO::syncObject");
}
@@ -141,6 +144,8 @@ IO::~IO()
{
traceClose();
closeFile();
+ if(created && deleteFilesOnExit)
+ deleteFile();
}
static bool isAbsolutePath(const char *name)
@@ -178,6 +183,8 @@ static JString getPath(const char *filen
bool IO::openFile(const char * name, bool readOnly)
{
+ ASSERT(!inCreateDatabase);
+
fileName = getPath(name);
for (int attempt = 0; attempt < 3; ++attempt)
@@ -198,6 +205,7 @@ bool IO::openFile(const char * name, boo
}
isReadOnly = readOnly;
+ created = false;
#ifndef _WIN32
signal (SIGXFSZ, SIG_IGN);
@@ -227,8 +235,8 @@ bool IO::createFile(const char *name)
for (int attempt = 0; attempt < 3; ++attempt)
{
- fileId = ::open (fileName,
- getWriteMode(attempt) | O_CREAT | O_RDWR | O_RANDOM | O_TRUNC | O_BINARY,
+ fileId = ::open (fileName.getString(),
+ getWriteMode(attempt) | O_CREAT | O_RDWR | O_RANDOM | O_EXCL | O_BINARY,
S_IREAD | S_IWRITE | S_IRGRP | S_IWGRP);
if (fileId >= 0)
@@ -252,7 +260,7 @@ bool IO::createFile(const char *name)
fcntl(fileId, F_SETLK, &lock);
#endif
#endif
-
+ created = true;
return fileId != -1;
}
=== modified file 'storage/falcon/IOx.h'
--- a/storage/falcon/IOx.h 2008-08-29 20:41:13 +0000
+++ b/storage/falcon/IOx.h 2008-09-04 10:49:57 +0000
@@ -107,9 +107,11 @@ public:
bool fatalError;
bool isReadOnly;
bool forceFsync;
+ bool created;
//private:
Dbb *dbb; // this is a crock and should be phased out
};
-
+extern bool deleteFilesOnExit;
+extern bool inCreateDatabase;
#endif // !defined(AFX_IO_H__6A019C19_A340_11D2_AB5A_0000C01D2301__INCLUDED_)
=== modified file 'storage/falcon/SQLException.h'
--- a/storage/falcon/SQLException.h 2008-04-22 09:19:03 +0000
+++ b/storage/falcon/SQLException.h 2008-09-04 10:49:57 +0000
@@ -69,7 +69,8 @@ enum SqlCode {
TABLESPACE_NOT_EXIST_ERROR = -35,
DEVICE_FULL = -36,
FILE_ACCESS_ERROR = -37,
- TABLESPACE_DATAFILE_EXIST_ERROR = -38
+ TABLESPACE_DATAFILE_EXIST_ERROR = -38,
+ RECOVERY_ERROR = -39
};
class DllExport SQLException {
=== modified file 'storage/falcon/SerialLogFile.cpp'
--- a/storage/falcon/SerialLogFile.cpp 2008-07-15 18:57:27 +0000
+++ b/storage/falcon/SerialLogFile.cpp 2008-09-04 10:49:57 +0000
@@ -76,16 +76,22 @@ SerialLogFile::SerialLogFile(Database *d
highWater = 0;
writePoint = 0;
forceFsync = false;
+ created = false;
sectorSize = database->serialLogBlockSize;
}
SerialLogFile::~SerialLogFile()
{
close();
+ if(created && deleteFilesOnExit)
+ unlink(fileName);
}
void SerialLogFile::open(JString filename, bool create)
{
+ if(!create)
+ ASSERT(!inCreateDatabase);
+
#ifdef _WIN32
handle = 0;
char pathName[1024];
@@ -96,7 +102,7 @@ void SerialLogFile::open(JString filenam
GENERIC_READ | GENERIC_WRITE,
0, // share mode
NULL, // security attributes
- (create) ? CREATE_ALWAYS : OPEN_ALWAYS,
+ (create) ? CREATE_NEW : OPEN_EXISTING,
FILE_FLAG_NO_BUFFERING | FILE_FLAG_RANDOM_ACCESS | FILE_FLAG_WRITE_THROUGH,
0);
@@ -123,7 +129,7 @@ void SerialLogFile::open(JString filenam
for (int attempt = 0; attempt < 3; ++attempt)
{
if (create)
- handle = ::open(filename, IO::getWriteMode(attempt) | O_RDWR | O_BINARY | O_CREAT, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+ handle = ::open(filename, IO::getWriteMode(attempt) | O_RDWR | O_BINARY | O_CREAT|O_EXCL, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
else
handle = ::open(filename, IO::getWriteMode(attempt) | O_RDWR | O_BINARY);
@@ -146,7 +152,10 @@ void SerialLogFile::open(JString filenam
#endif
if (create)
+ {
+ created = true;
zap();
+ }
}
void SerialLogFile::close()
=== modified file 'storage/falcon/SerialLogFile.h'
--- a/storage/falcon/SerialLogFile.h 2008-06-23 10:22:43 +0000
+++ b/storage/falcon/SerialLogFile.h 2008-09-04 10:49:57 +0000
@@ -51,6 +51,7 @@ public:
Mutex syncObject;
Database *database;
bool forceFsync;
+ bool created;
#ifdef _WIN32
void *handle;
=== modified file 'storage/falcon/StorageHandler.cpp'
--- a/storage/falcon/StorageHandler.cpp 2008-08-14 11:24:18 +0000
+++ b/storage/falcon/StorageHandler.cpp 2008-09-04 10:49:57 +0000
@@ -81,7 +81,7 @@ static const char *falconSchema [] = {
class Server;
extern Server* startServer(int port, const char *configFile);
-static StorageHandler *storageHandler;
+StorageHandler *storageHandler;
static char charTable[256];
static int init();
@@ -113,6 +113,11 @@ StorageHandler* getFalconStorageHandler(
return storageHandler;
}
+void freeFalconStorageHandler(void)
+{
+ delete storageHandler;
+ storageHandler = 0;
+}
void StorageHandler::setDataDirectory(const char *directory)
{
IO::setBaseDirectory(directory);
@@ -131,6 +136,8 @@ StorageHandler::StorageHandler(int lockS
syncObject.setName("StorageHandler::syncObject");
hashSyncObject.setName("StorageHandler::hashSyncObject");
dictionarySyncObject.setName("StorageHandler::dictionarySyncObject");
+ inCreateDatabase=false;
+ deleteFilesOnExit=false;
}
StorageHandler::~StorageHandler(void)
@@ -982,30 +989,49 @@ void StorageHandler::initialize(void)
}
catch (SQLException &e)
{
- // No point in creating a database if we got memory error.
- // On FILE_ACCESS_ERROR, an external application can temporarily lock the file.
- // In this both cases, trying to create database in this case could eventually
- // lead to "recreate" and data loss.
-
int err = e.getSqlcode();
-
- if (err == OUT_OF_MEMORY_ERROR || err == FILE_ACCESS_ERROR || err == VERSION_ERROR)
+
+ // If got one of following errors, just rethrow. No point in
+ // trying to create database.
+ if (err == OUT_OF_MEMORY_ERROR || err == FILE_ACCESS_ERROR ||
+ err == VERSION_ERROR || err == RECOVERY_ERROR)
throw;
- defaultDatabase->createDatabase();
- IO::deleteFile(FALCON_USER);
- IO::deleteFile(FALCON_TEMPORARY);
- dictionaryConnection = defaultDatabase->getOpenConnection();
- Statement *statement = dictionaryConnection->createStatement();
- JString createTableSpace = genCreateTableSpace(DEFAULT_TABLESPACE, FALCON_USER);
- statement->executeUpdate(createTableSpace);
-
- for (const char **ddl = falconSchema; *ddl; ++ddl)
- statement->executeUpdate(*ddl);
-
- statement->close();
- dictionaryConnection->commit();
+ try
+ {
+ createDatabase();
+ }
+ catch(SQLException &e2)
+ {
+ deleteFilesOnExit = true; // Cleanup created files
+
+ throw SQLError(e2.getSqlcode(),
+ "create database failed : %s \n" \
+ "prior open database failure was: %s",
+ e2.getText(),e.getText());
+ }
+ catch(...)
+ {
+ deleteFilesOnExit = true;
+ }
}
+}
+
+void StorageHandler::createDatabase(void)
+{
+ // Check all files are properly opened with O_CREAT
+ inCreateDatabase = true;
+ defaultDatabase->createDatabase();
+ dictionaryConnection = defaultDatabase->getOpenConnection();
+ Statement *statement = dictionaryConnection->createStatement();
+ JString createTableSpace = genCreateTableSpace(DEFAULT_TABLESPACE, FALCON_USER);
+ statement->executeUpdate(createTableSpace);
+
+ for (const char **ddl = falconSchema; *ddl; ++ddl)
+ statement->executeUpdate(*ddl);
+ statement->close();
+ dictionaryConnection->commit();
+ inCreateDatabase = false;
}
void StorageHandler::dropTempTables(void)
=== modified file 'storage/falcon/StorageHandler.h'
--- a/storage/falcon/StorageHandler.h 2008-08-14 11:24:18 +0000
+++ b/storage/falcon/StorageHandler.h 2008-09-04 10:49:57 +0000
@@ -53,6 +53,7 @@ class THD;
extern "C"
{
StorageHandler* getFalconStorageHandler(int lockSize);
+void freeFalconStorageHandler(void);
}
static const int databaseHashSize = 101;
@@ -126,6 +127,7 @@ public:
int closeConnections(THD* thd);
int dropDatabase(const char* path);
void initialize(void);
+ void createDatabase(void);
void dropTempTables(void);
void cleanFileName(const char* pathname, char* filename, int filenameLength);
JString genCreateTableSpace(const char* tableSpaceName, const char* filename, const char* comment = NULL);
=== modified file 'storage/falcon/ha_falcon.cpp'
--- a/storage/falcon/ha_falcon.cpp 2008-08-29 20:41:13 +0000
+++ b/storage/falcon/ha_falcon.cpp 2008-09-04 10:49:57 +0000
@@ -72,7 +72,7 @@ static const char *falcon_extensions[] =
NullS
};
-static StorageHandler *storageHandler;
+extern StorageHandler *storageHandler;
#define PARAMETER_UINT(_name, _text, _min, _default, _max, _flags, _update_function) \
uint falcon_##_name;
@@ -164,8 +164,7 @@ int StorageInterface::falcon_init(void *
StorageHandler::setDataDirectory(mysql_real_data_home);
- if (!storageHandler)
- storageHandler = getFalconStorageHandler(sizeof(THR_LOCK));
+ storageHandler = getFalconStorageHandler(sizeof(THR_LOCK));
falcon_hton->state = SHOW_OPTION_YES;
falcon_hton->db_type = DB_TYPE_FALCON;
@@ -216,13 +215,12 @@ int StorageInterface::falcon_init(void *
}
catch(SQLException &e)
{
- sql_print_error("Falcon: Exception '%s' during initialization",
- e.getText());
+ sql_print_error("Falcon: %s", e.getText());
error = true;
}
catch(...)
{
- sql_print_error(" Falcon: General exception in initialization");
+ sql_print_error("Falcon: General exception in initialization");
error = true;
}
@@ -230,8 +228,7 @@ int StorageInterface::falcon_init(void *
if (error)
{
// Cleanup after error
- delete storageHandler;
- storageHandler = 0;
+ falcon_deinit(0);
DBUG_RETURN(1);
}
@@ -242,8 +239,10 @@ int StorageInterface::falcon_init(void *
int StorageInterface::falcon_deinit(void *p)
{
if(storageHandler)
+ {
storageHandler->shutdownHandler();
-
+ freeFalconStorageHandler();
+ }
return 0;
}
@@ -2010,16 +2009,12 @@ void StorageInterface::freeActiveBlobs(v
void StorageInterface::shutdown(handlerton *htons)
{
- if(storageHandler)
- storageHandler->shutdownHandler();
+ falcon_deinit(0);
}
int StorageInterface::panic(handlerton* hton, ha_panic_function flag)
{
- if(storageHandler)
- storageHandler->shutdownHandler();
-
- return 0;
+ return falcon_deinit(0);
}
int StorageInterface::closeConnection(handlerton *hton, THD *thd)
| Thread |
|---|
| • bzr commit into mysql-6.0-falcon branch (vvaintroub:2808) Bug#39138 | Vladislav Vaintroub | 4 Sep |
