mbwaheed commented on a change in pull request #1055: SOLR-13932 Review 
directory locking and Blob interactions
URL: https://github.com/apache/lucene-solr/pull/1055#discussion_r356421777
 
 

 ##########
 File path: 
solr/core/src/java/org/apache/solr/store/blob/metadata/CorePushPull.java
 ##########
 @@ -268,7 +264,7 @@ public void pullUpdateFromBlob(long requestQueuedTimeMs, 
boolean waitForSearcher
                   }
 
 
 Review comment:
   Can't seem to comment on the unchanged lines, therefore, commenting here:
   
https://github.com/apache/lucene-solr/pull/1055/files?utf8=%E2%9C%93&diff=unified&w=1#diff-d86585f4b79b136f1575702bd7c61e38R226
 is not correct, it talks about incorrect understanding of directory locking.
   
   Pull is now an exclusive operation(i.e.no other pull or active indexing 
should be happening during this time). If local directory contents have changed 
we fail the pull. Therefore:
   
   1. After closing index writer we should obtain an exclusive lock on 
directory, similar to 
https://github.com/apache/lucene-solr/blob/1133bf98a5fd075173efecfb75a51493fceb62b3/solr/core/src/java/org/apache/solr/update/SolrIndexSplitter.java#L161
   
   2. We should close index writer and acquire write lock in the beginning of 
method so that the chances of directory changes(most likely background merges?) 
are even less. I am assuming Directory.obtainLock(IndexWriter.WRITE_LOCK_NAME) 
would prevent background merges from changing index directory. @yonik can you 
please chime in on this that understanding?
   
   3. The only purpose of temp directory, I see, is to prevent a partial 
download of index(transient network error) from corrupting the index directory. 
Otherwise we could have just downloaded directly into index directory. Comments 
on temp directory usage need to be updated to reflect that.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to