Merge lp:~oontvoo/akiban-server/bug_drop_fulltext_index into lp:~akiban-technologies/akiban-server/trunk

[Vy Nguyen]

fix drop fulltext index bug (bug 1163026) and bug1154336

[Resubmit with pre-requisite: lp:~oontvoo/akiban-server/new-trunk_do-maintenance]

[ Resubmit #2: Remove pre-req because it is in trunk]

[Nathan Williams (nwilliams)]

What is the IndexDef needed for?

I'm guessing bug1154336 is quite similar too. Perhaps dropTableInternal() needs to be in on the game.

review: Needs Information

[Vy Nguyen (oontvoo)]

PersisitStore.java - line 1554 (+/- a few lines)

It looks like the index needs to be 'attached' to an IndexDef.

[Nathan Williams (nwilliams)]

Only to delete the tree, which full texts don't have.

DDLFunctions has separate methods for dropping table and group indexes. Something different needs to happen for full texts (i.e. FullTextIndexService#dropIndex), so another entry point would be consistent and make sense.

[Nathan Williams (nwilliams)]

Actually, handling the distinction in PS#deleteIndexes() would let dropTableInternal() use it as well, so I'm fine with that too.

[Vy Nguyen (oontvoo)]

skip fulltext index in persistitStore.deleteIndexes()

review: Needs Resubmitting

[Nathan Williams (nwilliams)]

Since we're here, can you please fix the other bug I mentioned. We just need to call FTIS#dropIndex() for them.

review: Needs Fixing

[Vy Nguyen (oontvoo)]

I am having a compilation-error in

src/test/java/com/akiban/server/test/FailureOnStartupIT.java:[85,12] error: constructor OperatorStore in class OperatorStore cannot be applied to given types;

(I declared FTS as an argument in PersistStore's ctor, and other places where it's called.)

What did I miss?

review: Needs Information

[Vy Nguyen (oontvoo)]

Hm, never mind, found it!

[Nathan Williams (nwilliams)]

Look at FailureOnStartupIT.java, line 85. It extends OperatorStore so its constructor needs adjusted too.

[Vy Nguyen (oontvoo)]

Actually, it's more complicated than just calling FTS.dropIndex()

+ By the time deleteIndexes() is called, the index is no longer in AIS for FullTextIndexShared.forAIS() to look up. But that's not the 'complicated' part.

+ In order for dropIndex() to work, the index needs to be in 'indexes' map, which none of the current fulltext index is because no one puts it in there. The createIndex() method, which would do this, needs to be called manually.
This problem is solved in the do-maintenance branch. And I can certainly have it fix the bug you mentioned. (In fact, I tried merging this branch into it and the delete works correctly, which it wouldn't in this branch).

review: Needs Resubmitting

[Nathan Williams (nwilliams)]

> By the time deleteIndexes() is called, the index is no
> longer in AIS for FullTextIndexShared.forAIS() to look up

It does not need to be in the current AIS and probably shouldn't be, given that it's being removed. Look at the flow of the other drop methods (e.g. table, index). The AIS changes are performed through the SchemaManager before tree deletion is done. You can either hang onto the 'old' AIS or get it through the Index reference.

> In order for dropIndex() to work, the index needs to be in 'indexes' map,

It should be, and sounds like will be once your other branch is in, but doesn't need to be. The getIndex() method will populate the indexes map on demand. It creates a value cached on the AIS, which is going away in this case, but will work just fine.

review: Needs Fixing

[Vy Nguyen (oontvoo)]

clean up and more tests

review: Needs Resubmitting

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2637. By Vy Nguyen on 2013-04-17

use wait() in test instead of blindly sleep for a long time

[Nathan Williams (nwilliams)]

akserver.text.indexpath is already in both configuration-defaults and TestConfigService, so it shouldn't be needed in PSMYIT. Moving the interval setting into TestConfigService might be good, as it is now in 3 places.

The new getIndexToDrop() helper is misleading. It will throw NoSuchIndex if the cache hasn't been created (e.g. create a FT and then immediately drop). Using the existing method is fine. Attempting to optimize away the Info creation is just noise.

Otherwise looks good.

review: Needs Fixing

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2638. By Vy Nguyen on 2013-04-22

merge trunk, move configs to TestConfigService

2639. By Vy Nguyen on 2013-04-22

edge cases in removeTrees()

[Vy Nguyen (oontvoo)]

A few *subtle* changes:

- in removeTrees():
  The change is to ensure dropIndex is called on a particular index only once (for the same reason described in the comment in the code)

- in FullTextIndexServiceIT
  Because fullText index deletions were not done properly (which is what the branch is trying to fix), these test methods could call FTS.dropIndex() whenever they want.
  Now that the bug is fixed, these should not eagerly drop the index (simply to remove it from the tree( because tearDown() would then not be able to find the indices to really drop them.

- About your comment on getIndexToDrop()
  The error message could change, but using the existing method would require some mechanism to ensure the downstream methods lookup the index in the old AIS, not the new one, because the index is no longer in there. And all that is just not necessary because all we really want to do here is to remove the index from the cache and delete relating document(s).

  Deleting the index right after creating it is similar to using it right after creation. Unexpected things could happen, given that the index is populated asynchronously .

[Vy Nguyen (oontvoo)]

review: Needs Resubmitting

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2640. By Vy Nguyen on 2013-04-22

remove unused imports in FullTextIndexServiceIT

[Nathan Williams (nwilliams)]

There is a helper to return the 'owned' full text indexes: UserTable#getOwnFullTextIndexes().

Multiple tests still set the indexpath and working intervals.

The re-insertion in FullTextIndexServiceIT is highly suspect. Either the test needs rewritten or tearDown's assumptions need changed, though I didn't see what tearDown was actually refering too.

We can't rely on the contents of the cache to determine whether or not there is work to do. It is a cache and can be empty at any time. The decision *must* be made on the basis of the presence in the AIS. Getting it from the old AIS is simple, just pass it the old AIS.

We also can't say that "unexpected things could happen" if you drop an index. Not a very good user experience. Instead, we should ensure that the correct thing always happens. I see that the existing dropIndex() attempts to remove a 'promise' for population. That isn't enough, given that they could be concurrent. Instead, a shared claim should be made during the population or maintenance steps (see LockService). An exclusive claim is already taken during DROP processing.

review: Needs Fixing

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2641. By Vy Nguyen on 2013-04-22

use getownFullTextIndex

2642. By Vy Nguyen on 2013-04-22

clean up

[Vy Nguyen (oontvoo)]

> The re-insertion in FullTextIndexServiceIT is highly suspect. Either the test
> needs rewritten or tearDown's assumptions need changed, though I didn't see
> what tearDown was actually refering too.
>

tearDown() is called automatically at the end of the test class to drop all tables/indices/schemas, etc. It does not make any assumption, so I don't think we should change anything about that, though I agree testDelete() isn't exactly straightforward.

The test is intended to test the the populate-worker, NOT the deletion of the index per se.
In other words, we wanted to make sure that when a fulltext index is created, an entry about it is inserted into a tree for later processing, but if the index is dropped before the time scheduled for population, we wants to make sure the entry is removed from that tree. Otherwise, when the worker wakes up, it will populate a non-existing index.

> We can't rely on the contents of the cache to determine whether or not there
> is work to do. It is a cache and can be empty at any time. The decision *must*
> be made on the basis of the presence in the AIS. Getting it from the old AIS
> is simple, just pass it the old AIS.

I believe I misused the word 'cache' in the resubmit comment. What I really meant was FTS.dropIndex() only needed to remove the entry about the index from the hashmap (ie., `indexes`), and to delete relating document(s).

It wasn't checking some cache to see if the given index exists in there. The index must exist in `indexes` if it was properly populated.

Now that I thought about it, throwing an error on line 44 is 'over-reacting'. That wouldn't really be an error. Probably just a warning in the debug log would be sufficient. The only time this would happen is, as you said, when the index is deleted before it was populated, and because all that FTS.dropIndex() is responsible for doing is to clean up the stuff that only exists *after* the population, there is nothing to do here (other than removing the 'promise' for population, of course).

[Nathan Williams (nwilliams)]

> The test is intended to test the the populate-worker, NOT
> the deletion of the index per se. In other words, we wanted
> to make sure that when a fulltext index is created, an entry
> about it is inserted into a tree for later processing, but if
> the index is dropped before the time scheduled for population,
> we wants to make sure the entry is removed from that tree

Since, I believe, we should claim a lock on the table when performing the populate or update, this would be a good time to check that the index still exists. If it doesn't, the entry from the background tree can be removed and all work skipped. As I mentioned in the previous comment, removing the entry from the dropIndex() processing isn't enough.

> I believe I misused the word 'cache' in the resubmit comment.

Cache is the correct word. The IndexInfo is stored in a cache that is attached to the AIS. We can make no assumptions about its contents or lifetime.

> The only time this would happen is, as you said, when the index is
> deleted before it was populated,

No. The best way to think of it is really as a cache that can be cleared at any time. In the current implementation, the cache will be empty any time there is *any* DDL, not just one related to the table. This is because a brand new AIS each time.time.

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2643. By Vy Nguyen on 2013-04-22

clean up

2644. By Vy Nguyen on 2013-04-22

pass Index as an argument to dropIndex to correctly determine the treename

[Vy Nguyen (oontvoo)]

- construct a simple* FulLTextIndexShared/Info for deleting the docs, if none already exists.
This should get rid of the NoSuchIndexException in getIndexToDrop()

*simple* because we don't want to
(1) cache the object in AIS (No one is ever going to use the cached value)
(2) compute operator plan for lookup the rows of the index (same reason as above)
(3) All that we wants here is the path to the document file(s)

review: Needs Resubmitting

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2645. By Vy Nguyen on 2013-04-22

delete fulltext index

[Nathan Williams (nwilliams)]

As I mentioned a number of comments ago, optimizing away the cache generation is noise. The unlikely case is it doesn't exist. Less code is generally better.

Please address the issues in my previous comments most importantly locking but also duplicate settings tests.

review: Needs Fixing

[Vy Nguyen (oontvoo)]

> As I mentioned a number of comments ago, optimizing away the cache generation
> is noise.
> The unlikely case is it doesn't exist.

That is not a *little* optimisation. The current getIndex() would cause a bunch of stuff to be done including building a list of rowTypes and computing plans to lookup related rows. Why would we want to do all that just to delete the index?

> Please address the issues in my previous comments most importantly locking but
> also duplicate settings tests.

I did. All the settings are gone, only TestConfigService still sets the interval now.

What about locking that you were getting at? If that's about an entry being the tree possibly being modified elsewhere, too, it's addressed in the ww_bug branch that I just put in.

[Nathan Williams (nwilliams)]

> That is not a *little* optimisation. The current getIndex()
> would cause a bunch of stuff to be done including building
> a list of rowTypes and computing plans to lookup related
> rows. Why would we want to do all that just to delete the
> index?

The amount of time that the getIndex() would take compared to the rest of the DROP processing is miniscule. Yes, it creates a little garbage but that's about it.

Why? As I said earlier, less code is generally better. Less to understand, maintain, document, and test. In a scenario where we can have a single code path vs two, and the cost is a little garbage in uncommon scenarios, I'll always choose the former.

> I did. All the settings are gone, only TestConfigService still
> sets the interval now.

OptimizerTestBase.java:136
OptimizerTestBase.java:137
OptimizerTestBase.java:138

TestConfigService also declares TEXT_INDEX_PATH_KEY, which duplicates FTISI#INDEX_PATH_PROPERTY.

> What about locking that you were getting at?

As I mentioned previously, removing the key in FTSISI#deleteFromTree() is not enough. What happens if population is occurring the same time as deletion? Slightly before? Slightly after?

review: Needs Fixing

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2646. By Vy Nguyen on 2013-04-26

use default settings in OptimizerTestBase

[Vy Nguyen (oontvoo)]

- configs: removed

- locking: this is addressed the ww_bug. I am not going to touch it here.

- I still don't see your arguments about more vs less code. To use the current code (ie., getIndex), we'd need to pass the old AIS to it, which'd lead to a series of change in its callers to take the AIS all the way from [Index|Table| *]DDL.java.
That doesn't look any less code.

At least, the current change is local.

[Nathan Williams (nwilliams)]

Thanks for the tweaks.

> - locking: this is addressed the ww_bug. I am not going to
> touch it here.

The other branch addresses part, but not all unfortunately. Note the specific cases I said in the previous comment (i.e. writes before populate is done). Doing this in another branch is fine.

> - I still don't see your arguments about more vs less code.
> To use the current code (ie., getIndex), we'd need to pass the
> old AIS to it,

In this instance, it's already immediately available: idx.getIndexedTable().getAIS(). If that isn't palatable, there are only two callers of dropIndex() so passing it in isn't onerous.

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2647. By Vy Nguyen on 2013-04-29

use the old AIS when neccessary

[Vy Nguyen (oontvoo)]

use the old AIS when neccessary

review: Needs Resubmitting

[lp:~oontvoo/akiban-server/bug_drop_fulltext_index]

2648. By Vy Nguyen on 2013-04-29

remove unused import accidentally added

[Nathan Williams (nwilliams)]

Thanks.

review: Approve

=== modified file 'src/main/java/com/akiban/server/service/dxl/BasicDDLFunctions.java'
--- src/main/java/com/akiban/server/service/dxl/BasicDDLFunctions.java	2013-04-14 04:32:58 +0000
+++ src/main/java/com/akiban/server/service/dxl/BasicDDLFunctions.java	2013-04-29 16:13:27 +0000
@@ -1032,8 +1032,10 @@
             Collection<Index> indexes = new HashSet<>();
             for(String indexName : indexNamesToDrop) {
                 Index index = table.getIndex(indexName);
-                if(index == null) {
-                    throw new NoSuchIndexException (indexName);
+                if(index == null
+                        && !(table.isUserTable()
+                                && ((index = ((UserTable)table).getFullTextIndex(indexName)) != null))) {
+                        throw new NoSuchIndexException (indexName);
                 }
                 if(index.isPrimaryKey()) {
                     throw new ProtectedIndexException(indexName, table.getName());

=== modified file 'src/main/java/com/akiban/server/service/text/FullTextIndexInfo.java'
--- src/main/java/com/akiban/server/service/text/FullTextIndexInfo.java	2013-04-01 19:04:26 +0000
+++ src/main/java/com/akiban/server/service/text/FullTextIndexInfo.java	2013-04-29 16:13:27 +0000
@@ -264,10 +264,11 @@
         return indexer;
     }
 
-    // TODO: _Lookup plan to get rows just from a row being updated.
-
     public void deletePath() {
         File path = shared.getPath();
+        // no doc to delete
+        if (!path.exists() || path.listFiles() == null)
+            return;
         for (File f : path.listFiles()) {
             f.delete();
         }

=== modified file 'src/main/java/com/akiban/server/service/text/FullTextIndexInfosImpl.java'
--- src/main/java/com/akiban/server/service/text/FullTextIndexInfosImpl.java	2013-03-22 20:05:57 +0000
+++ src/main/java/com/akiban/server/service/text/FullTextIndexInfosImpl.java	2013-04-29 16:13:27 +0000
@@ -21,7 +21,6 @@
 import com.akiban.ais.model.IndexName;
 import com.akiban.qp.operator.QueryContext;
 import com.akiban.qp.rowtype.RowType;
-import com.akiban.qp.rowtype.Schema;
 import com.akiban.server.error.FullTextQueryParseException;
 import com.akiban.server.service.session.Session;
 
@@ -30,7 +29,6 @@
 import org.apache.lucene.search.Query;
 
 import java.io.File;
-import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -41,7 +39,7 @@
     @Override
     public Query parseQuery(QueryContext context, IndexName name, 
                             String defaultField, String query) {
-        FullTextIndexInfo index = getIndex(context.getSession(), name);
+        FullTextIndexInfo index = getIndex(context.getSession(), name, null);
         if (defaultField == null) {
             defaultField = index.getDefaultFieldName();
         }
@@ -58,15 +56,16 @@
 
     @Override
     public RowType searchRowType(Session session, IndexName name) {
-        FullTextIndexInfo index = getIndex(session, name);
+        FullTextIndexInfo index = getIndex(session, name, null);
         return index.getHKeyRowType();
     }
 
     protected abstract AkibanInformationSchema getAIS(Session session);
     protected abstract File getIndexPath();
 
-    protected FullTextIndexInfo getIndex(Session session, IndexName name) {
-        AkibanInformationSchema ais = getAIS(session);
+    protected FullTextIndexInfo getIndex(Session session, IndexName name, AkibanInformationSchema ais) {
+        if (ais == null)
+            ais = getAIS(session);
         FullTextIndexInfo info;
         synchronized (indexes) {
             FullTextIndexShared shared = indexes.get(name);

=== modified file 'src/main/java/com/akiban/server/service/text/FullTextIndexService.java'
--- src/main/java/com/akiban/server/service/text/FullTextIndexService.java	2013-04-08 19:43:08 +0000
+++ src/main/java/com/akiban/server/service/text/FullTextIndexService.java	2013-04-29 16:13:27 +0000
@@ -17,6 +17,7 @@
 
 package com.akiban.server.service.text;
 
+import com.akiban.ais.model.FullTextIndex;
 import com.akiban.ais.model.IndexName;
 import com.akiban.qp.operator.Cursor;
 import com.akiban.qp.operator.QueryContext;
@@ -50,7 +51,7 @@
      * @return An array of available background works
      */
     public List<? extends BackgroundWork> getBackgroundWorks();
-    public void dropIndex(Session session, IndexName name);
+    public void dropIndex(Session session, FullTextIndex name);
     public Cursor searchIndex(QueryContext context, IndexName name, 
                               Query query, int limit);
 }

=== modified file 'src/main/java/com/akiban/server/service/text/FullTextIndexServiceImpl.java'
--- src/main/java/com/akiban/server/service/text/FullTextIndexServiceImpl.java	2013-04-17 17:51:15 +0000
+++ src/main/java/com/akiban/server/service/text/FullTextIndexServiceImpl.java	2013-04-29 16:13:27 +0000
@@ -18,7 +18,7 @@
 package com.akiban.server.service.text;
 
 import com.akiban.ais.model.AkibanInformationSchema;
-import com.akiban.ais.model.Index;
+import com.akiban.ais.model.FullTextIndex;
 import com.akiban.ais.model.IndexName;
 import com.akiban.ais.model.TableName;
 import com.akiban.ais.model.UserTable;
@@ -34,7 +34,6 @@
 import com.akiban.qp.rowtype.HKeyRowType;
 import com.akiban.qp.util.HKeyCache;
 import com.akiban.server.error.AkibanInternalException;
-import com.akiban.server.service.BackgroundObserver;
 import com.akiban.server.service.BackgroundWork;
 import com.akiban.server.service.BackgroundWorkBase;
 import com.akiban.server.service.Service;
@@ -63,8 +62,6 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.Observable;
-import java.util.Observer;
 import java.util.Timer;
 import java.util.TimerTask;
 
@@ -114,7 +111,7 @@
     /* FullTextIndexService */
 
     private long createIndex(Session session, IndexName name) {
-        FullTextIndexInfo index = getIndex(session, name);
+        FullTextIndexInfo index = getIndex(session, name, null);
         try {
             return populateIndex(session, index);
         }
@@ -123,12 +120,10 @@
         }
     }
 
-    @Override
-    public void dropIndex(Session session, IndexName name) {
-        
+    void deleteFromTree(Session session, IndexName name)
+    {
         try
         {
-            // see if there exists a promise for populating this index
             Exchange ex = getPopulateExchange(session);
             ex.clear().append(name.getSchemaName())
                       .append(name.getTableName())
@@ -136,22 +131,29 @@
             
             if (ex.traverse(Key.Direction.EQ, true, 0))
                 ex.fetchAndRemove();
-
-            FullTextIndexInfo index = getIndex(session, name);
-            index.deletePath();
-            synchronized (indexes) {
-                indexes.remove(name);
-            }
         }
         catch (PersistitException e)
         {
             throw new AkibanInternalException("Error while removing index", e);
         }
     }
+    
+    @Override
+    public void dropIndex(Session session, FullTextIndex idx) {
+        // delete 'promise' for population, if any
+        deleteFromTree(session, idx.getIndexName());
+        
+        // delete documents
+        FullTextIndexInfo idxInfo = getIndex(session, idx.getIndexName(), idx.getIndexedTable().getAIS());
+        idxInfo.deletePath();
+        synchronized (indexes) {    
+            indexes.remove(idx.getIndexName());
+        }
+    }
 
     @Override
     public Cursor searchIndex(QueryContext context, IndexName name, Query query, int limit) {
-        FullTextIndexInfo index = getIndex(context.getSession(), name);
+        FullTextIndexInfo index = getIndex(context.getSession(), name, null);
         try {
             return index.getSearcher().search(context, index.getHKeyRowType(), 
                                               query, limit);
@@ -172,6 +174,7 @@
     @Override
     public void start() {
         persistitStore = store.getPersistitStore();
+        persistitStore.setFullTextService(this);
         enableUpdateWorker();
         enablePopulateWorker();
     }
@@ -255,7 +258,7 @@
     {
         try
         {
-            FullTextIndexInfo indexInfo = getIndex(session, name);
+            FullTextIndexInfo indexInfo = getIndex(session, name, null);
             StoreAdapter adapter = session.get(StoreAdapter.STORE_ADAPTER_KEY);
             if (adapter == null)
                 adapter = new PersistitAdapter(indexInfo.getSchema(),

=== modified file 'src/main/java/com/akiban/server/service/text/FullTextIndexShared.java'
--- src/main/java/com/akiban/server/service/text/FullTextIndexShared.java	2013-03-22 20:05:57 +0000
+++ src/main/java/com/akiban/server/service/text/FullTextIndexShared.java	2013-04-29 16:13:27 +0000
@@ -27,7 +27,6 @@
 import org.apache.lucene.store.FSDirectory;
 
 import java.io.*;
-import java.util.List;
 import java.util.Set;
 
 public class FullTextIndexShared implements CacheValueGenerator<FullTextIndexInfo>, Closeable

=== modified file 'src/main/java/com/akiban/server/store/PersistitStore.java'
--- src/main/java/com/akiban/server/store/PersistitStore.java	2013-04-15 16:20:25 +0000
+++ src/main/java/com/akiban/server/store/PersistitStore.java	2013-04-29 16:13:27 +0000
@@ -18,6 +18,7 @@
 package com.akiban.server.store;
 
 import com.akiban.ais.model.*;
+import com.akiban.ais.model.Index.IndexType;
 import com.akiban.qp.operator.StoreAdapter;
 import com.akiban.qp.persistitadapter.OperatorBasedRowCollector;
 import com.akiban.qp.persistitadapter.PersistitAdapter;
@@ -41,6 +42,7 @@
 import com.akiban.server.service.config.ConfigurationService;
 import com.akiban.server.service.lock.LockService;
 import com.akiban.server.service.session.Session;
+import com.akiban.server.service.text.FullTextIndexService;
 import com.akiban.server.service.transaction.TransactionService;
 import com.akiban.server.service.tree.TreeLink;
 import com.akiban.server.service.tree.TreeService;
@@ -130,6 +132,8 @@
 
     private int deferredIndexKeyLimit = MAX_INDEX_TRANCHE_SIZE;
 
+    private FullTextIndexService fullTextService;
+
     private RowDataValueCoder valueCoder;
 
     // Each row change has a 'uniqueChangeId', stored in the 'maintenance.full_text' table
@@ -157,6 +161,11 @@
         this.transactionService = transactionService;
     }
 
+    public void setFullTextService(FullTextIndexService service)
+    {
+        fullTextService = service;
+    }
+
      // --- for tracking changes 
     private Exchange getChangeExchange(Session session) throws PersistitException
     {   
@@ -1890,6 +1899,14 @@
     @Override
     public void removeTrees(Session session, Table table) {
         Collection<TreeLink> treeLinks = new ArrayList<>();
+
+        // delete all fulltext indexes
+        if (table.isUserTable())
+        {
+            for (FullTextIndex idx : ((UserTable)table).getOwnFullTextIndexes())
+                fullTextService.dropIndex(session, idx);
+        }
+
         // Add all index trees
         final Collection<TableIndex> tableIndexes = table.isUserTable() ? ((UserTable)table).getIndexesIncludingInternal() : table.getIndexes();
         final Collection<GroupIndex> groupIndexes = table.getGroupIndexes();
@@ -1931,6 +1948,13 @@
     public void deleteIndexes(final Session session, final Collection<? extends Index> indexes) {
         List<TreeLink> links = new ArrayList<>(indexes.size());
         for(Index index : indexes) {
+            // no trees to drop
+            if (index.getIndexType() == IndexType.FULL_TEXT)
+            {
+                fullTextService.dropIndex(session, (FullTextIndex)index);
+                indexes.remove(index);
+                continue;
+            }
             final IndexDef indexDef = index.indexDef();
             if(indexDef == null) {
                 throw new IllegalStateException("indexDef is null for index: " + index);

=== modified file 'src/main/java/com/akiban/sql/aisddl/IndexDDL.java'
--- src/main/java/com/akiban/sql/aisddl/IndexDDL.java	2013-03-27 04:42:10 +0000
+++ src/main/java/com/akiban/sql/aisddl/IndexDDL.java	2013-04-29 16:13:27 +0000
@@ -89,7 +89,7 @@
             }
             // if we can't find the index, set tableName to null
             // to flag not a user table index. 
-            if (table.getIndex(indexName) == null) {
+            if (table.getIndex(indexName) == null && table.getFullTextIndex(indexName) == null) {
                 tableName = null;
             }
             // Check the group associated to the table for the 

=== modified file 'src/test/java/com/akiban/server/service/config/TestConfigService.java'
--- src/test/java/com/akiban/server/service/config/TestConfigService.java	2013-03-22 20:05:57 +0000
+++ src/test/java/com/akiban/server/service/config/TestConfigService.java	2013-04-29 16:13:27 +0000
@@ -22,6 +22,7 @@
 import com.akiban.server.service.plugins.Plugin;
 import com.akiban.server.service.plugins.PluginsFinder;
 
+import com.akiban.server.service.text.FullTextIndexServiceImpl;
 import java.io.File;
 import java.util.Collection;
 import java.util.Collections;
@@ -60,6 +61,8 @@
     protected Map<String, String> loadProperties() {
         Map<String, String> ret = new HashMap<>(super.loadProperties());
         makeDataDirectory();
+        ret.put(FullTextIndexServiceImpl.UPDATE_INTERVAL, Long.toString(1000));
+        ret.put(FullTextIndexServiceImpl.POPULATE_DELAY_INTERVAL, Long.toString(1000));
         ret.put(DATA_PATH_KEY, dataDirectory.getAbsolutePath());
         ret.put(TEXT_INDEX_PATH_KEY, dataDirectory.getAbsolutePath());
         final int bufferSize = Integer.parseInt(ret.get(BUFFER_SIZE_KEY));

=== modified file 'src/test/java/com/akiban/server/service/text/FullTextIndexServiceIT.java'
--- src/test/java/com/akiban/server/service/text/FullTextIndexServiceIT.java	2013-04-15 19:21:09 +0000
+++ src/test/java/com/akiban/server/service/text/FullTextIndexServiceIT.java	2013-04-29 16:13:27 +0000
@@ -45,8 +45,6 @@
 public class FullTextIndexServiceIT extends ITBase
 {
     public static final String SCHEMA = "test";
-    private static final long updateInterval = 1000L; // for testing
-    private static final long populateDelayInterval = 1000L;
     protected FullTextIndexService fullText;
     protected Schema schema;
     protected PersistitAdapter adapter;
@@ -63,15 +61,6 @@
                 .bindAndRequire(FullTextIndexService.class, FullTextIndexServiceImpl.class);
     }
 
-    @Override
-    protected Map<String, String> startupConfigProperties() {
-        Map<String, String> properties = new HashMap<>();
-        properties.put("akserver.text.indexpath", "/tmp/aktext");
-        properties.put(FullTextIndexServiceImpl.UPDATE_INTERVAL, Long.toString(updateInterval));
-        properties.put(FullTextIndexServiceImpl.POPULATE_DELAY_INTERVAL, Long.toString(populateDelayInterval));
-        return properties;
-    }
-
     @Before
     public void createData() {
         c = createTable(SCHEMA, "c",
@@ -179,12 +168,6 @@
                          assertEquals (0, n);
                      } 
                  });
-        
-        // drop all the indices
-        Session session = new SessionServiceImpl().createSession();
-        for (FullTextIndex idx : expecteds)
-            fullTextImpl.dropIndex(session, idx.getIndexName());
-        session.close();
     }
 
 
@@ -217,29 +200,9 @@
 
         // <3> delete 2 of them
         Session session = new SessionServiceImpl().createSession();
-        boolean sawNPE = false;
-        try
-        {
-            fullTextImpl.dropIndex(session, expecteds[0].getIndexName());
-        }
-        catch (NullPointerException e) // NPE is expected because, the index hasn't been
-        {                              // populated yet. But we're not testing that!
-            sawNPE = true;
-        }
-        assertTrue("NPE should have happened", sawNPE);
-        
+        deleteFullTextIndex(serviceManager(), expecteds[0].getIndexName());
+        deleteFullTextIndex(serviceManager(), expecteds[1].getIndexName());
 
-        sawNPE = false;
-        try
-        {
-            fullTextImpl.dropIndex(session, expecteds[1].getIndexName());
-        }
-        catch (NullPointerException e) // NPE is expected because, the index hasn't been
-        {                              // populated yet. But we're not testing that!
-            sawNPE = true;
-        }
-        assertTrue("NPE should have happened", sawNPE);
-        
         // <4> check that the tree only has one entry now (ie., epxecteds2[2]
         traverse(fullTextImpl,
                  new Visitor()
@@ -259,13 +222,11 @@
                          assertEquals (1, n);
                      } 
                  });
-
+        
         // wake the worker up to do its job
         fullTextImpl.enablePopulateWorker();
         WaitFunctionHelpers.waitOn(fullText.getBackgroundWorks());
         
-        // drop the remaining index
-        fullTextImpl.dropIndex(session, expecteds[2].getIndexName());
         session.close();
     }
 

=== modified file 'src/test/java/com/akiban/server/test/ApiTestBase.java'
--- src/test/java/com/akiban/server/test/ApiTestBase.java	2013-04-10 18:32:00 +0000
+++ src/test/java/com/akiban/server/test/ApiTestBase.java	2013-04-29 16:13:27 +0000
@@ -767,6 +767,12 @@
         return ddl().getAIS(session()).getGroup(groupName).getIndex(indexName);
     }
 
+    protected final void deleteFullTextIndex(ServiceManager sm, IndexName name)
+    {
+        ddl().dropTableIndexes(session(), name.getFullTableName(), Arrays.asList(name.getName()));
+        updateAISGeneration();
+    }
+    
     protected final FullTextIndex createFullTextIndex(ServiceManager sm, String schema, String table, String indexName, String... indexCols) {
         AkibanInformationSchema tempAIS = createIndexInternal(schema, table, indexName, "FULL_TEXT(" + Strings.join(Arrays.asList(indexCols), ",") + ")");
         Index tempIndex = tempAIS.getUserTable(schema, table).getFullTextIndex(indexName);

=== modified file 'src/test/java/com/akiban/sql/optimizer/OptimizerTestBase.java'
--- src/test/java/com/akiban/sql/optimizer/OptimizerTestBase.java	2013-04-01 19:04:26 +0000
+++ src/test/java/com/akiban/sql/optimizer/OptimizerTestBase.java	2013-04-29 16:13:27 +0000
@@ -117,7 +117,7 @@
     }
 
      
-    protected static ServiceManager createServiceManager(Map<String, String> startupConfigProperties)
+    protected static ServiceManager createServiceManager()
     {
         return new GuicedServiceManager(serviceBindingsProvider());
     }
@@ -130,19 +130,10 @@
                                         FullTextIndexServiceImpl.class);
     }
     
-
-    protected static Map<String, String> startupConfigProperties() {
-        Map<String, String> properties = new HashMap<>();
-        properties.put("akserver.text.indexpath", "/tmp/aktext");
-        properties.put(FullTextIndexServiceImpl.UPDATE_INTERVAL, Long.toString(updateInterval));
-        properties.put(FullTextIndexServiceImpl.POPULATE_DELAY_INTERVAL, Long.toString(populateDelayInterval));
-        return properties;
-    }
-    
     private static ServiceManager prepareServices()
     {
         System.setProperty("akiban.home", System.getProperty("user.home"));
-        ServiceManager ret = createServiceManager(startupConfigProperties());
+        ServiceManager ret = createServiceManager();
         ret.startServices();
         
         return ret;

=== modified file 'src/test/java/com/akiban/sql/pg/PostgresServerMiscYamlIT.java'
--- src/test/java/com/akiban/sql/pg/PostgresServerMiscYamlIT.java	2013-04-11 05:18:38 +0000
+++ src/test/java/com/akiban/sql/pg/PostgresServerMiscYamlIT.java	2013-04-29 16:13:27 +0000
@@ -23,6 +23,8 @@
 import com.akiban.server.service.is.BasicInfoSchemaTablesService;
 import com.akiban.server.service.is.BasicInfoSchemaTablesServiceImpl;
 import com.akiban.server.service.servicemanager.GuicedServiceManager;
+import com.akiban.server.service.text.FullTextIndexService;
+import com.akiban.server.service.text.FullTextIndexServiceImpl;
 import com.akiban.sql.NamedParamsTestBase;
 import com.akiban.sql.embedded.EmbeddedJDBCService;
 import com.akiban.sql.embedded.EmbeddedJDBCServiceImpl;
@@ -79,7 +81,8 @@
     protected GuicedServiceManager.BindingsConfigurationProvider serviceBindingsProvider() {
         return super.serviceBindingsProvider()
                 .bindAndRequire(BasicInfoSchemaTablesService.class, BasicInfoSchemaTablesServiceImpl.class)
-                .bindAndRequire(EmbeddedJDBCService.class, EmbeddedJDBCServiceImpl.class);
+                .bindAndRequire(EmbeddedJDBCService.class, EmbeddedJDBCServiceImpl.class)
+                .bindAndRequire(FullTextIndexService.class, FullTextIndexServiceImpl.class);
     }
 
     @Override

=== added file 'src/test/resources/com/akiban/sql/pg/yaml/functional/test-drop-fulltext.yaml'
--- src/test/resources/com/akiban/sql/pg/yaml/functional/test-drop-fulltext.yaml	1970-01-01 00:00:00 +0000
+++ src/test/resources/com/akiban/sql/pg/yaml/functional/test-drop-fulltext.yaml	2013-04-29 16:13:27 +0000
@@ -0,0 +1,45 @@
+# test drop full_text index
+---
+- CreateTable: t(id int, name varchar(224));
+---
+- Statement: CREATE INDEX idx on t(full_text(name));
+---
+- Statement: SELECT fulltext_maintenance_wait();
+---
+- Statement: INSERT INTO t values (1, 'bar1'), (2, 'bar2'), (3, 'bar3'), (4, 'bar1');
+---
+- Statement: SELECT fulltext_maintenance_wait();
+---
+- Statement: SELECT id from t where full_text_search(name='bar1');
+- output: [[1], [4]]
+---
+- Statement: DROP INDEX t.idx;
+---
+- DropTable: t;
+---
+- CreateTable: t2(id int, name varchar(32));
+---
+- Statement: INSERT INTO t2 VALUES (1, 'John'), (2, 'Sally');
+---
+- Statement: CREATE INDEX t2_idx on t2(full_text(name));
+---
+- Statement: SELECT fulltext_maintenance_wait();
+---
+- Statement: SELECT * FROM t2 WHERE FULL_TEXT_SEARCH(name, 'Jo*');
+- row_count: 0
+---
+- DropTable: t2;
+---
+- CreateTable: t2(id INT NOT NULL PRIMARY KEY, name VARCHAR(32) COLLATE en_ci);
+---
+- Statement: INSERT INTO t2 VALUES (1, 'John'), (2, 'Sally');
+---
+- Statement: CREATE INDEX t2_idx on t2(full_text(name));
+---
+- Statement: SELECT fulltext_maintenance_wait()
+---
+- Statement: SELECT * FROM t2 WHERE FULL_TEXT_SEARCH(name, 'Jo*');
+- output: [[1, 'John']]
+---
+- DropTable: t2;
+...
\ No newline at end of file