bzr commit into mysql-6.0-falcon-team branch (vvaintroub:2892) Bug#38186

[Vladislav Vaintroub]

#At file:///G:/bzr/mysql-6.0-falcon-team/

 2892 Vladislav Vaintroub	2008-10-31
       Bug #38186: CREATE TABLESPACE failed when invoked immediately following a
       DROP TABLESPACE statement that used the same tablespace name.
      
       Fixed a long-standing problem that tablespace file was present even after "drop
tablespace" successfully 
      completed.  The problem was worked around previously, this patch contains refined
solution. 
      Previously, file was closed and deleted when the gopher thread processed
dropTableSpace record.
       Now  the file is removed earlier, in the client thread . Gopher thread only closes
the file descriptor later. 
      
      This technique ensures that 
      - datafile does not exist after "drop tablespace" is completed
      - file descriptor remains valid as long as it can be used by commited , 
write-incomplete transactions.
      
      On  Windows, removing an open file requires FILE_SHARE_DELETE flag for CreateFile.
Also file has to be renamed to 
      a unique name before deletion. This functionality is implemented  in posix-alike
winOpen() and winUnlink()  functions
      
      This patch also contains missing test case for Bug#38186.
added:
  mysql-test/suite/falcon/r/falcon_bug_38186.result
  mysql-test/suite/falcon/t/falcon_bug_38186.test
modified:
  storage/falcon/IO.cpp
  storage/falcon/StorageHandler.cpp
  storage/falcon/TableSpaceManager.cpp
  storage/falcon/TableSpaceManager.h

=== added file 'mysql-test/suite/falcon/r/falcon_bug_38186.result'
--- a/mysql-test/suite/falcon/r/falcon_bug_38186.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/falcon/r/falcon_bug_38186.result	2008-10-31 00:29:13 +0000
@@ -0,0 +1,2 @@
+*** Bug #38186 ***
+SET @@storage_engine = 'Falcon';

=== added file 'mysql-test/suite/falcon/t/falcon_bug_38186.test'
--- a/mysql-test/suite/falcon/t/falcon_bug_38186.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/falcon/t/falcon_bug_38186.test	2008-10-31 00:29:13 +0000
@@ -0,0 +1,29 @@
+--source include/have_falcon.inc
+#
+# Bug #38186: 
+# CREATE TABLESPACE can fail when invoked immediately following a
+# DROP TABLESPACE statement that used the same tablespace name.
+#
+--echo *** Bug #38186 ***
+
+# ----------------------------------------------------- #
+# --- Initialisation                                --- #
+# ----------------------------------------------------- #
+let $engine = 'Falcon';
+eval SET @@storage_engine = $engine;
+# ----------------------------------------------------- #
+# --- Test                                          --- #
+# ----------------------------------------------------- #
+--disable_query_log
+let $i = 100;
+while($i)
+{
+  eval CREATE TABLESPACE ts ADD DATAFILE 'file.fts' ENGINE = $engine;
+  CREATE TABLE t(i int) TABLESPACE ts;
+  INSERT INTO t values(1);
+  DROP TABLE t;
+  eval DROP TABLESPACE ts ENGINE= $engine;
+  dec $i;
+}
+--enable_query_log
+

=== modified file 'storage/falcon/IO.cpp'
--- a/storage/falcon/IO.cpp	2008-09-11 10:56:00 +0000
+++ b/storage/falcon/IO.cpp	2008-10-31 00:29:13 +0000
@@ -117,6 +117,19 @@ static char baseDir[PATH_MAX+1]={0};
 bool deleteFilesOnExit = false;
 bool inCreateDatabase = false;
 
+#ifdef _WIN32
+static int winUnlink(const char *file);
+static int winOpen(const char *filename, int flags,...);
+#endif
+
+#ifdef _WIN32
+#define POSIX_OPEN_FILE winOpen
+#define POSIX_UNLINK_FILE winUnlink
+#else
+#define POSIX_OPEN_FILE   ::open
+#define POSIX_UNLINK_FILE ::unlink
+#endif
+
 #ifdef _DEBUG
 #undef THIS_FILE
 static const char THIS_FILE[]=__FILE__;
@@ -186,7 +199,7 @@ bool IO::openFile(const char * name, boo
 	ASSERT(!inCreateDatabase);
 
 	fileName = getPath(name);
-	fileId = ::open (fileName, (readOnly) ? (O_RDONLY | O_BINARY) : (O_RDWR | O_BINARY));
+	fileId = POSIX_OPEN_FILE(fileName, (readOnly) ? (O_RDONLY | O_BINARY) : (O_RDWR |
O_BINARY));
 
 	if (fileId < 0)
 		{
@@ -241,7 +254,7 @@ bool IO::createFile(const char *name)
 	Log::debug("IO::createFile: creating file \"%s\"\n", name);
 
 	fileName = getPath(name);
-	fileId = ::open (fileName.getString(),O_CREAT | O_RDWR | O_RANDOM | O_EXCL | O_BINARY,
+	fileId = POSIX_OPEN_FILE (fileName.getString(),O_CREAT | O_RDWR | O_RANDOM | O_EXCL |
O_BINARY,
 						S_IREAD | S_IWRITE | S_IRGRP | S_IWGRP);
 
 
@@ -557,6 +570,7 @@ void IO::writeHeader(Hdr *header)
 void IO::deleteFile()
 {
 	deleteFile(fileName);
+	fileName="";
 }
 
 int IO::pread(int64 offset, int length, UCHAR* buffer)
@@ -675,10 +689,132 @@ void IO::sync(void)
 		traceOperation(TRACE_SYNC_END);
 }
 
+#ifdef _WIN32
+#define FALCON_DELETED_FILE "fdf"
+
+/*
+	The only safe way to delete file on Windows without invalidating open file 
+	handles is that:
+	- rename file to be deleted to a unique temporary name.
+	- open file it with FILE_FLAG_DELETE_ON_CLOSE
+	- close the file handle
+	Temp file will disappear as soon as last handle on it is closed.
+	This works only if files are opened with FILE_SHARE_DELETE flag.
+*/
+
+static int winUnlink(const char *file)
+{
+	DWORD attributes = GetFileAttributes(file);
+
+	// Bail out, if file does not exist.
+	if (attributes == INVALID_FILE_ATTRIBUTES)
+		return -1;
+
+	// If file is a symbolic link, just delete the link, but not the link target
+	if (attributes & FILE_ATTRIBUTE_REPARSE_POINT)
+		{
+		if(DeleteFile(file))
+			return 0;
+		return -1;
+		}
+
+	// Rename the file to unique name, then open with FILE_FLAG_DELETE_ON_CLOSE
+	// and close.
+	char  tmpDir[MAX_PATH];
+	strncpy(tmpDir, file, sizeof(tmpDir)-1);
+	char *p = strrchr(tmpDir ,SEPARATOR);
+	if (p)
+		*p = 0;
+	else
+		strcpy(tmpDir,".");
+
+	char  tmpFile[MAX_PATH];
+	if (GetTempFileName(tmpDir, FALCON_DELETED_FILE, 0, tmpFile))
+		{
+		if (MoveFileEx(file, tmpFile, MOVEFILE_REPLACE_EXISTING))
+			{
+			HANDLE hFile = CreateFile(tmpFile, 0,
+				FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
+				NULL, OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, NULL);
+
+			if (hFile != INVALID_HANDLE_VALUE)
+				{
+				CloseHandle(hFile);
+				return 0;
+				}
+			}
+		else
+			DeleteFile(tmpFile);
+		}
+	
+	// Something went wrong. Try DeleteFile(), even if it can invalidate
+	// open file handles.
+	if(DeleteFile(file))
+		return 0;
+
+	return -1;
+}
+
+/* 
+	A wrapper for Posix open(). The reason it is there is the FILE_SHARE_DELETE 
+	flag used in CreateFile, which allows for  posixly-correct unlink
+	(that works with open files)
+*/
+static int winOpen(const char *filename, int flags,...)
+{
+	DWORD access;
+	if (flags & O_WRONLY)
+		access = GENERIC_WRITE;
+	else if (flags & O_RDWR)
+		access = GENERIC_READ|GENERIC_WRITE;
+	else
+		access = GENERIC_READ;
+
+	DWORD disposition;
+	if (flags & O_CREAT)
+		disposition = CREATE_NEW;
+	else
+		disposition = OPEN_EXISTING;
+
+	DWORD attributes;
+	if (flags & O_RANDOM)
+		attributes = FILE_FLAG_RANDOM_ACCESS;
+	else
+		attributes = FILE_ATTRIBUTE_NORMAL;
+
+	HANDLE hFile = CreateFile(filename, access, 
+		FILE_SHARE_DELETE, NULL, disposition, attributes, NULL);
+	
+	if (hFile == INVALID_HANDLE_VALUE)
+		{
+		switch(GetLastError())
+			{
+			case ERROR_ACCESS_DENIED:
+				errno = EACCES;
+				break;
+			case ERROR_FILE_NOT_FOUND:
+			case ERROR_PATH_NOT_FOUND:
+				errno = ENOENT;
+				break;
+			default:
+				errno = EINVAL;
+				break;
+			}
+		return -1;
+		}
+	return _open_osfhandle((intptr_t)hFile, 
+		flags & (_O_APPEND|_O_RDONLY|_O_TEXT));
+}
+
+#endif
+
 void IO::deleteFile(const char* fileName)
 {
-	JString path = getPath(fileName);
-	unlink(path.getString());
+	if(fileName && *fileName)
+		{
+		JString path = getPath(fileName);
+		POSIX_UNLINK_FILE(path.getString());
+		}
 }
 
 void IO::tracePage(Bdb* bdb)
@@ -810,3 +946,4 @@ uint16 IO::computeChecksum(Page *page, s
 	return (uint16) sum;
 
 }
+

=== modified file 'storage/falcon/StorageHandler.cpp'
--- a/storage/falcon/StorageHandler.cpp	2008-10-01 03:13:44 +0000
+++ b/storage/falcon/StorageHandler.cpp	2008-10-31 00:29:13 +0000
@@ -508,10 +508,6 @@ int StorageHandler::createTablespace(con
 	TableSpaceManager *tableSpaceManager =
 		dictionaryConnection->database->tableSpaceManager;
 
-	if (!tableSpaceManager->waitForPendingDrop(filename, 10))
-		// file still exists after waiting for 10 seconds
-		return  StorageErrorTableSpaceDataFileExist;
-
 	try
 		{
 		JString cmd = genCreateTableSpace(tableSpaceName, filename, comment);

=== modified file 'storage/falcon/TableSpaceManager.cpp'
--- a/storage/falcon/TableSpaceManager.cpp	2008-09-22 09:24:39 +0000
+++ b/storage/falcon/TableSpaceManager.cpp	2008-10-31 00:29:13 +0000
@@ -58,7 +58,6 @@ TableSpaceManager::TableSpaceManager(Dat
 	memset(nameHash, 0, sizeof(nameHash));
 	memset(idHash, 0, sizeof(nameHash));
 	tableSpaces = NULL;
-	pendingDrops = 0;
 	syncObject.setName("TableSpaceManager::syncObject");
 }
 
@@ -298,13 +297,14 @@ void TableSpaceManager::dropTableSpace(T
 			break;
 			}
 			
-	pendingDrops++;
 	syncObj.unlock();
+	tableSpace->active = false;
+	JString filename = tableSpace->dbb->fileName;
 
 	database->serialLog->logControl->dropTableSpace.append(tableSpace,
transaction);
 	database->commitSystemTransaction();
+	IO::deleteFile(filename);
 
-	tableSpace->active = false;
 }
 
 void TableSpaceManager::reportStatistics(void)
@@ -370,12 +370,10 @@ void TableSpaceManager::expungeTableSpac
 			}
 
 	sync.unlock();
-	tableSpace->dropTableSpace();
+	//File already deleted, just close the file descriptor
+	tableSpace->close();
 	delete tableSpace;
 
-	sync.lock(Exclusive);
-	if(pendingDrops >0)
-		pendingDrops--;
 }
 
 void TableSpaceManager::reportWrites(void)
@@ -563,28 +561,5 @@ void TableSpaceManager::getTableSpaceFil
 		infoTable->setNull(37);			// EXTRA
 		infoTable->putRecord();
 		}
-}
-
-
-// Wait for specified amount of time for a  file to be deleted.
-// Don't wait if pendingDrops count is 0.
-//
-// The function returns true, if wait was successfull, i.e file does not exist
-//(anymore)
-bool TableSpaceManager::waitForPendingDrop(const char  *filename, int seconds)
-{
-	bool fileExists;
-
-	do
-		{
-		fileExists = IO::doesFileExist(filename);
-		if (fileExists && pendingDrops > 0 && seconds-- > 0)
-			Thread::getThread("TransactionManager::waitForPendingDrop")->sleep(1000);
-		else
-			break;
-		}
-	while(true);
-
-	return !fileExists;
 }
 

=== modified file 'storage/falcon/TableSpaceManager.h'
--- a/storage/falcon/TableSpaceManager.h	2008-09-22 09:24:39 +0000
+++ b/storage/falcon/TableSpaceManager.h	2008-10-31 00:29:13 +0000
@@ -63,7 +63,6 @@ public:
 	void			reportWrites(void);
 	void			redoCreateTableSpace(int id, int nameLength, const char* name, int
fileNameLength, const char* fileName, int type, TableSpaceInit* tsInit);
 	void			initialize(void);
-	bool			waitForPendingDrop(const char *filename, int seconds);
 
 	Database	*database;
 	TableSpace	*tableSpaces;
@@ -71,7 +70,6 @@ public:
 	TableSpace	*idHash[TS_HASH_SIZE];
 	SyncObject	syncObject;
 	void postRecovery(void);
-	int pendingDrops;
 };
 
 #endif //
!defined(AFX_TABLESPACEMANAGER_H__BD1D39F6_2201_4136_899C_7CB106E99B8C__INCLUDED_)