Commit while addIndexes is in progress

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Commit while addIndexes is in progress

Ning Li-3
Hi,

Should we guard against the case when commit() is called during addIndexes?
Otherwise, errors such as a file does not exist could happen during commit.

Cheers,
Ning Li

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Commit while addIndexes is in progress

Michael McCandless-2

Hmm, I think we should.

What should it "mean" when you call commit(), while another thread is  
in the middle of addIndexes?

We could 1) disallow it (throw an exception if you try), 2) allow it  
but block until addIndexes is done, 3) allow it but commit all changes  
up until when addIndexes was first called ... anything else?

I think there're similar problems with calling optimize() while  
addIndexes is in progress... I think we should disallow that?

Mike

Ning Li wrote:

> Hi,
>
> Should we guard against the case when commit() is called during  
> addIndexes?
> Otherwise, errors such as a file does not exist could happen during  
> commit.
>
> Cheers,
> Ning Li
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Commit while addIndexes is in progress

Ning Li-3
> What should it "mean" when you call commit(), while another thread is in the
> middle of addIndexes?
>
> We could 1) disallow it (throw an exception if you try), 2) allow it but
> block until addIndexes is done, 3) allow it but commit all changes up until
> when addIndexes was first called ... anything else?

I don't have a strong preference. One reasoning is that commit itself
calls flush
and waits for flush to finish. So is 2) more consistent that commit waits for
addIndexes to finish?

We should also disallow concurrent addIndexes, right?


> I think there're similar problems with calling optimize() while addIndexes
> is in progress... I think we should disallow that?

Optimize waits for addIndexes to finish? I think it's useful to allow addIndexes
during maybeMerge and optimize, no?

Cheers,
Ning Li

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Commit while addIndexes is in progress

Yonik Seeley-2
In reply to this post by Michael McCandless-2
On Fri, Jul 11, 2008 at 2:38 PM, Michael McCandless
<[hidden email]> wrote:
>
> Hmm, I think we should.
>
> What should it "mean" when you call commit(), while another thread is in the
> middle of addIndexes?

Seems like either all or none of the segments in addIndexes should be committed.

> We could 1) disallow it (throw an exception if you try), 2) allow it but
> block until addIndexes is done, 3) allow it but commit all changes up until
> when addIndexes was first called ... anything else?

I think either 2 or 3 is reasonable, and we could avoid defining
which, as long as we do something reasonable.  If the application
really cares which is done, they should be synchronizing at a higher
level themselves.

> I think there're similar problems with calling optimize() while addIndexes
> is in progress... I think we should disallow that?

I don't think it should throw an exception... it seems like any kind
of constraint like that should be handled by IndexWriter.

-Yonik

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Commit while addIndexes is in progress

Yonik Seeley-2
In reply to this post by Ning Li-3
On Fri, Jul 11, 2008 at 3:27 PM, Ning Li <[hidden email]> wrote:
> We should also disallow concurrent addIndexes, right?

Hmmm, the current implementation looks like it won't currently won't
work correctly (docWriter.resumeAllThreads() being called while
another thread is calling addIndexes, etc).

-Yonik

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Commit while addIndexes is in progress

Michael McCandless-2

Yonik Seeley wrote:

> On Fri, Jul 11, 2008 at 3:27 PM, Ning Li <[hidden email]> wrote:
>> We should also disallow concurrent addIndexes, right?
>
> Hmmm, the current implementation looks like it won't currently won't
> work correctly (docWriter.resumeAllThreads() being called while
> another thread is calling addIndexes, etc).

Actually docWriter.pause/resumeAllThreads uses ref counting, so, these  
won't step on each other (ie threads are not actually resumed until  
all addIndexes calls have called resumeAllThreads).

But, the start/commit/rollbackTransaction calls will not be happy if  
more than one thread tries to open a transaction at once.

I'll fix it so a 2nd addIndexes call, while one is already running,  
will wait until the first one finishes before it then starts.

Mike

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Commit while addIndexes is in progress

Michael McCandless-2
In reply to this post by Yonik Seeley-2

Yonik Seeley wrote:

> On Fri, Jul 11, 2008 at 2:38 PM, Michael McCandless
> <[hidden email]> wrote:
>>
>> Hmm, I think we should.
>>
>> What should it "mean" when you call commit(), while another thread  
>> is in the
>> middle of addIndexes?
>
> Seems like either all or none of the segments in addIndexes should  
> be committed.
>
>> We could 1) disallow it (throw an exception if you try), 2) allow  
>> it but
>> block until addIndexes is done, 3) allow it but commit all changes  
>> up until
>> when addIndexes was first called ... anything else?
>
> I think either 2 or 3 is reasonable, and we could avoid defining
> which, as long as we do something reasonable.  If the application
> really cares which is done, they should be synchronizing at a higher
> level themselves.

I agree, the app should synchronize if it cares which of 2 or 3.  For  
now I'll do 2...

>
>> I think there're similar problems with calling optimize() while  
>> addIndexes
>> is in progress... I think we should disallow that?
>
> I don't think it should throw an exception... it seems like any kind
> of constraint like that should be handled by IndexWriter.

Agreed, we shouldn't throw exceptions under any of these.

Mike

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Commit while addIndexes is in progress

Michael McCandless-2
In reply to this post by Ning Li-3

Ning Li wrote:

>> I think there're similar problems with calling optimize() while  
>> addIndexes
>> is in progress... I think we should disallow that?
>
> Optimize waits for addIndexes to finish? I think it's useful to  
> allow addIndexes
> during maybeMerge and optimize, no?

OK I agree it would be nice to let optimize run concurrently with  
addIndexes.  They are both just doing merges, anyway, so it should not  
be too hard.

I'll open an issue for these...

Mike

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]