MergeScheduler does not provide a sync() method in its abstract class. Therefore, an implementation could be used that would not derive from or provide a sync() method
If it did, this would be a more than acceptable solution instead of putting the method on the IndexWriter. (i potentially want to allow configuration to specify what MergeScheduler to use, and if sync() isn't specified on the base class, i can't reliably use it (will have to do instanceof checks, and a new concurrent based scheduler not derived from ConcurrentMergeScheduler breaks this)
We can add a sync() method to MergeScheduler or you do it on your side. There are only two MS today: CMS and SMS. If you're going to write your own CMS variant, then you can have it extend CMS, or define your own interface with a sync() method, which will delegate that call to whatever MS it wraps. Again, the way I see it, you know which MS you instantiate, and therefore you should be able to declare its type and use whatever methods it has. If you want something configurable, I suggest we move sync() to MS (w/ a default impl of doing nothing, for back-compat).
What do you think?
... another thread calling addDocument triggering a merge
This will only happen if you use autoCommit=true, right? You can use autoCommit=true, and when you call commit(), use the one that waits for merges (the one that does not exist yet ), and sync all your addDoc threads with that current commit thread (make sure you use a ReadWriteLock, or otherwise all your addDocs are going to sync on each other).
this will allow blocking adding more documents until background merges are complete periodically
What's "periodically"? If you're using autoCommit=false, those merges cannot happen suddenly. There are only 4 methods that can trigger them (I think?) and you can sync on calling them w/ the rest of the addDoc threads.
Don't get me wrong - I just play the devil's advocate here. Recently, we've had this thread http://www.nabble.com/ReadOnly-IndexReaders-td23787255.html - a naive question on adding an ever more naive method to IndexReader - isReadOnly. If you read it, you'll see that it wasn't accepted at the end, because we've understood it will just try to cover a design flaw of your system.
Therefore I'm just trying to verify whether this method is absolutely necessary, and there aren't any "design decisions" we should make in our apps to better handle that case.
Having said that, I'm not a committer and if one decides it's important enough, one would take this issue and commit it. I just enjoy the discussion for now (hope you too ).