fix nasty drop / dropIndex bugs

8 files changed, 143 insertions, 39 deletions

diff --git a/db/clientcursor.cpp b/db/clientcursor.cpp
index eb72b69d1ce..5a6dece5a46 100644
--- a/db/clientcursor.cpp
+++ b/db/clientcursor.cpp
@@ -58,6 +58,26 @@ void removedKey(const DiskLoc& btreeLoc, int keyPos) {
 // TODO!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 }
 
+/* todo: this implementation is incomplete.  we use it as a prefix for dropDatabase, which 
+         works fine as the prefix will end with '.'.  however, when used with drop and 
+		 deleteIndexes, this could take out cursors that belong to something else -- if you 
+		 drop "foo", currently, this will kill cursors for "foobar".
+*/
+void ClientCursor::invalidate(const char *nsPrefix) { 
+	vector<ClientCursor*> toDelete;
+
+	int len = strlen(nsPrefix);
+	assert( len > 0 && strchr(nsPrefix, '.') );
+	for( ByLoc::iterator i = byLoc.begin(); i != byLoc.end(); ++i ) {
+		ClientCursor *cc = i->second;
+		if( strncmp(nsPrefix, cc->ns.c_str(), len) == 0 )
+			toDelete.push_back(i->second);
+	}
+
+	for( vector<ClientCursor*>::iterator i = toDelete.begin(); i != toDelete.end(); ++i ) 
+		delete (*i);
+}
+
 /* must call this on a delete so we clean up the cursors. */
 void aboutToDelete(const DiskLoc& dl) { 
 	vector<ClientCursor*> toAdvance;
diff --git a/db/clientcursor.h b/db/clientcursor.h
index c3ec58fe649..015b2e2c4b3 100644
--- a/db/clientcursor.h
+++ b/db/clientcursor.h
@@ -52,6 +52,11 @@ public:
 	auto_ptr< set<string> > filter; // which fields query wants returned
 	Message originalMessage; // this is effectively an auto ptr for data the matcher points to.
 
+	/* Get rid of cursors for namespaces that begin with nsprefix. 
+	   Used by drop, deleteIndexes, dropDatabase.
+	*/
+	static void invalidate(const char *nsPrefix);
+
 	static bool erase(CursorId id) { 
 		ClientCursor *cc = find(id);
 		if( cc ) {
@@ -65,7 +70,7 @@ public:
 		CCById::iterator it = clientCursorsById.find(id);
 		if( it == clientCursorsById.end() ) { 
 			if( warn ) 
-				cout << "ClientCursor::find(): cursor not found in map " << id << '\n';
+				OCCASIONALLY cout << "ClientCursor::find(): cursor not found in map " << id << " (ok after a drop)\n";
 			return 0;
 		}
 		return it->second;
@@ -77,9 +82,5 @@ public:
 	   */
 	void updateLocation();
 
-//private:
-//	void addToByLocation(DiskLoc cl);
 	void cleanupByLocation(DiskLoc loc);
-//public:
-//	ClientCursor *nextAtThisLocation;
 };
diff --git a/db/jsobj.h b/db/jsobj.h
index 152ad82189c..135db6e4319 100644
--- a/db/jsobj.h
+++ b/db/jsobj.h
@@ -206,7 +206,8 @@ class JSObj {
 		details = new Details();
 		details->_objdata = data;
 		details->_objsize = *((int*) data);
-		assert( details->_objsize < 1024 * 1024 * 16 );
+		assert( details->_objsize > 0 );
+		assert( details->_objsize <= 1024 * 1024 * 16 );
 		details->refCount = ifree ? 1 : -1;
 	}
 public:
diff --git a/db/namespace.h b/db/namespace.h
index 7f7fac876c3..0469e128f5d 100644
--- a/db/namespace.h
+++ b/db/namespace.h
@@ -93,6 +93,19 @@ public:
 		return s;
 	}
 
+	string indexName() { // e.g. "ts_1"
+		JSObj io = info.obj();
+		return io.getStringField("name");
+	}
+
+	/* gets not our namespace name (indexNamespace for that), 
+	   but the collection we index, its name.
+	   */
+	string parentNS() {
+		JSObj io = info.obj();
+		return io.getStringField("ns");
+	}
+
 	/* delete this index.  does NOT celan up the system catalog
 	   (system.indexes or system.namespaces) -- only NamespaceIndex.
 	*/
diff --git a/db/pdfile.cpp b/db/pdfile.cpp
index 5b68175d4d7..70f88909802 100644
--- a/db/pdfile.cpp
+++ b/db/pdfile.cpp
@@ -18,11 +18,10 @@
 
 /* 
 todo: 
-_ manage deleted records.  bucket?
-_ use deleted on inserts!
-_ quantize allocations
 _ table scans must be sequential, not next/prev pointers
-_ regex support
+_ coalesce deleted
+
+_ disallow system* manipulations from the client.
 */
 
 #include "stdafx.h"
@@ -34,6 +33,7 @@ _ regex support
 #include "btree.h"
 #include <algorithm>
 #include <list>
+#include "query.h"
 
 const char *dbpath = "/data/db/";
 
@@ -644,12 +644,41 @@ auto_ptr<Cursor> findTableScan(const char *ns, JSObj& order) {
 
 void aboutToDelete(const DiskLoc& dl);
 
-/* delete this index.  does NOT celan up the system catalog
+/* drop a collection/namespace */
+void dropNS(string& nsToDrop) {
+	assert( strstr(nsToDrop.c_str(), ".system.") == 0 );
+	{
+		// remove from the system catalog
+		JSObjBuilder b;
+		b.append("name", nsToDrop.c_str());
+		JSObj cond = b.done(); // { name: "colltodropname" }
+		string system_namespaces = client->name + ".system.namespaces";
+		int n = deleteObjects(system_namespaces.c_str(), cond, false, true);
+		wassert( n == 1 );
+	}
+	// remove from the catalog hashtable
+	client->namespaceIndex.kill(nsToDrop.c_str());
+}
+
+/* delete this index.  does NOT clean up the system catalog
    (system.indexes or system.namespaces) -- only NamespaceIndex.
 */
 void IndexDetails::kill() { 
-	string ns = indexNamespace();
-	client->namespaceIndex.kill(ns.c_str());
+	string ns = indexNamespace(); // e.g. foo.coll.$ts_1
+
+	{
+		// clean up in system.indexes
+		JSObjBuilder b;
+		b.append("name", indexName().c_str());
+		b.append("ns", parentNS().c_str());
+		JSObj cond = b.done(); // e.g.: { name: "ts_1", ns: "foo.coll" }
+		string system_indexes = client->name + ".system.indexes";
+		int n = deleteObjects(system_indexes.c_str(), cond, false, true);
+		wassert( n == 1 );
+	}
+
+	dropNS(ns);
+	//	client->namespaceIndex.kill(ns.c_str());
 	head.setInvalid();
 	info.setInvalid();
 }
@@ -661,6 +690,11 @@ void IndexDetails::kill() {
 */
 void IndexDetails::getKeysFromObject(JSObj& obj, set<JSObj>& keys) { 
 	JSObj keyPattern = info.obj().getObjectField("key");
+	if( keyPattern.objsize() == 0 ) {
+		cout << keyPattern.toString() << endl;
+		cout << info.obj().toString() << endl;
+		assert(false);
+	}
 	JSObjBuilder b;
 	JSObj key = obj.extractFields(keyPattern, b);
 	if( key.isEmpty() )
@@ -976,8 +1010,9 @@ DiskLoc DataFileMgr::insert(const char *ns, const void *buf, int len, bool god)
 		tabletoidxns = io.getStringField("ns");  // table it indexes
 		JSObj key = io.getObjectField("key");
 		if( name == 0 || *name == 0 || tabletoidxns == 0 || key.isEmpty() || key.objsize() > 2048 ) { 
-			cout << "user warning: bad add index attempt name:" << (name?name:"") << " ns:" << 
-				(tabletoidxns?tabletoidxns:"") << " ourns:" << ns << endl;
+			cout << "user warning: bad add index attempt name:" << (name?name:"") << "\n  ns:" << 
+				(tabletoidxns?tabletoidxns:"") << "\n  ourns:" << ns;
+			cout << "\n  idxobj:" << io.toString() << endl;
 			return DiskLoc();
 		}
 		tableToIndex = nsdetails(tabletoidxns);
@@ -1076,3 +1111,23 @@ void pdfileInit() {
 //	namespaceIndex.init(dbpath);
 	theDataFileMgr.init(dbpath);
 }
+
+#include "clientcursor.h"
+
+void dropDatabase(const char *ns) { 
+	// ns is of the form "<dbname>.$cmd"
+	char cl[256];
+	nsToClient(ns, cl);
+	problem() << "dropDatabase " << cl << endl;
+	assert( client->name == cl );
+
+	/* important: kill all open cursors on the database */
+	string prefix(cl);
+	prefix += '.';
+	ClientCursor::invalidate(prefix.c_str());
+
+	clients.erase(cl);
+	delete client; // closes files
+	client = 0;
+	_deleteDataFiles(cl);
+}
diff --git a/db/pdfile.h b/db/pdfile.h
index 346d93c64fa..89dcd602d33 100644
--- a/db/pdfile.h
+++ b/db/pdfile.h
@@ -39,6 +39,9 @@ class Extent;
 class Record;
 class Cursor;
 
+void dropDatabase(const char *ns);
+void dropNS(string& dropNs);
+
 /*---------------------------------------------------------------------*/ 
 
 class PDFHeader;
@@ -463,17 +466,6 @@ inline void _deleteDataFiles(const char *client) {
 	}
 }
 
-inline void dropDatabase(const char *ns) { 
-	char cl[256];
-	nsToClient(ns, cl);
-	problem() << "dropDatabase " << cl << endl;
-	assert( client->name == cl );
-	clients.erase(cl);
-	delete client; // closes files
-	client = 0;
-	_deleteDataFiles(cl);
-}
-
 inline NamespaceIndex* nsindex(const char *ns) { 
 	DEV { 
 		char buf[256];
diff --git a/db/query.cpp b/db/query.cpp
index 8281f8d84fc..61ebf048cc5 100644
--- a/db/query.cpp
+++ b/db/query.cpp
@@ -190,20 +190,25 @@ fail:
 	return auto_ptr<Cursor>();
 }
 
-void deleteObjects(const char *ns, JSObj pattern, bool justOne) {
-	if( strstr(ns, ".system.") ) {
-		if( strstr(ns, ".system.namespaces") ){ 
+/* ns:      namespace, e.g. <client>.<collection>
+   pattern: the "where" clause / criteria
+   justOne: stop after 1 match
+*/
+int deleteObjects(const char *ns, JSObj pattern, bool justOne, bool god) {
+	if( strstr(ns, ".system.") && !god ) {
+		/*if( strstr(ns, ".system.namespaces") ){ 
 			cout << "info: delete on system namespace " << ns << '\n';
 		}
 		else if( strstr(ns, ".system.indexes") ) {
 			cout << "info: delete on system namespace " << ns << '\n';
 		}
-		else { 
+		else*/ { 
 			cout << "ERROR: attempt to delete in system namespace " << ns << endl;
-			return;
+			return -1;
 		}
 	}
 
+	int nDeleted = 0;
 	JSMatcher matcher(pattern);
 	JSObj order;
 	auto_ptr<Cursor> c = getIndexCursor(ns, pattern, order);
@@ -236,12 +241,15 @@ void deleteObjects(const char *ns, JSObj pattern, bool justOne) {
 			_tempDelLoc = rloc;
 
 			theDataFileMgr.deleteRecord(ns, r, rloc);
+			nDeleted++;
 			tempd = temp;
 			if( justOne )
-				return;
+				break;
 			c->checkLocation();
 		}
 	}
+
+	return nDeleted;
 }
 
 struct Mod { 
@@ -676,19 +684,30 @@ inline bool _runCommands(const char *ns, JSObj& jsobj, stringstream& ss, BufBuil
 		}
 		else if( strcmp( e.fieldName(), "drop") == 0 ) { 
 			valid = true;
-			string dropNs = us + '.' + e.valuestr();
-			NamespaceDetails *d = nsdetails(dropNs.c_str());
-			cout << "CMD: drop " << dropNs << endl;
+			string nsToDrop = us + '.' + e.valuestr();
+			NamespaceDetails *d = nsdetails(nsToDrop.c_str());
+			cout << "CMD: drop " << nsToDrop << endl;
 			if( d == 0 ) {
 				anObjBuilder.append("errmsg", "ns not found");
 			}
 			else if( d->nIndexes != 0 ) {
+				// client is supposed to drop the indexes first
 				anObjBuilder.append("errmsg", "ns has indexes (not permitted on drop)");
 			}
 			else {
 				ok = true;
-				anObjBuilder.append("ns", dropNs.c_str());
+				anObjBuilder.append("ns", nsToDrop.c_str());
+				ClientCursor::invalidate(nsToDrop.c_str());
+				dropNS(nsToDrop);
+				/*
+				{
+					JSObjBuilder b;
+					b.append("name", dropNs.c_str());
+					JSObj cond = b.done(); // { name: "colltodropname" }
+					deleteObjects("system.namespaces", cond, false, true);
+				}
 				client->namespaceIndex.kill(dropNs.c_str());
+				*/
 			}
 		}
 		else if( strcmp( e.fieldName(), "validate") == 0 ) { 
@@ -715,7 +734,10 @@ inline bool _runCommands(const char *ns, JSObj& jsobj, stringstream& ss, BufBuil
 			if( d ) {
 				Element f = jsobj.findElement("index");
 				if( !f.eoo() ) { 
-					// delete a specific index
+
+					ClientCursor::invalidate(toDeleteNs.c_str());
+
+					// delete a specific index or all?
 					if( f.type() == String ) { 
 						const char *idxName = f.valuestr();
 						if( *idxName == '*' && idxName[1] == 0 ) { 
@@ -745,7 +767,7 @@ inline bool _runCommands(const char *ns, JSObj& jsobj, stringstream& ss, BufBuil
 								for( int i = x; i < d->nIndexes; i++ )
 									d->indexes[i] = d->indexes[i+1];
 								ok=true;
-								cout << "  alpha implementation, space not reclaimed" << endl;
+								cout << "  alpha implementation, space not reclaimed\n";
 							} else { 
 								cout << "deleteIndexes: " << idxName << " not found" << endl;
 							}
diff --git a/db/query.h b/db/query.h
index a59e61419d4..562a2ee4e39 100644
--- a/db/query.h
+++ b/db/query.h
@@ -89,6 +89,6 @@ QueryResult* runQuery(Message&, const char *ns, int ntoskip, int ntoreturn,
 					  stringstream&);
 
 void updateObjects(const char *ns, JSObj updateobj, JSObj pattern, bool upsert, stringstream& ss);
-void deleteObjects(const char *ns, JSObj pattern, bool justOne);
+int deleteObjects(const char *ns, JSObj pattern, bool justOne, bool god=false);
 
 #include "clientcursor.h"