bzr commit into mysql-6.0-falcon branch (vvaintroub:2808) Bug#39138

[Vladislav Vaintroub]

#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)