lucene-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [lucene-solr] yonik commented on a change in pull request #1055: SOLR-13932 Review directory locking and Blob interactions
Date Tue, 03 Dec 2019 16:51:02 GMT
yonik 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_r353298728
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/store/blob/metadata/CorePushPull.java
 ##########
 @@ -96,81 +96,77 @@ protected CoreContainer getContainerFromServerMetadata(ServerSideMetadata
solrSe
      */
     public BlobCoreMetadata pushToBlobStore(String currentMetadataSuffix, String newMetadataSuffix)
throws Exception {
       long startTimeMs = System.currentTimeMillis();
+      SolrCore solrCore = container.getCore(pushPullData.getCoreName());
+      if (solrCore == null) {
+        throw new Exception("Can't find core " + pushPullData.getCoreName());
+      }
+
       try {
-        SolrCore solrCore = container.getCore(pushPullData.getCoreName());
-        if (solrCore == null) {
-          throw new Exception("Can't find core " + pushPullData.getCoreName());
+        // Creating the new BlobCoreMetadata as a modified clone of the existing one
+        BlobCoreMetadataBuilder bcmBuilder = new BlobCoreMetadataBuilder(blobMetadata, solrServerMetadata.getGeneration());
+
+        /*
+         * Removing from the core metadata the files that are stored on the blob store but
no longer needed.
+         *
+         * When this method is executed, the content of the index on Blob is to be replaced
with the local content.
+         * The assumption (or normal flow) is for local to refresh from Blob, update locally
then push the
+         * changes to Blob.
+         * When merges happen locally the update has to mark old segment files for delete
(and for example the
+         * previous "segments_N" is to be deleted as a new higher generation segment has
been created).
+         */
+        for (BlobCoreMetadata.BlobFile d : resolvedMetadataResult.getFilesToDelete()) {
+            bcmBuilder.removeFile(d);
+            BlobCoreMetadata.BlobFileToDelete bftd = new BlobCoreMetadata.BlobFileToDelete(d,
System.currentTimeMillis());
+            bcmBuilder.addFileToDelete(bftd);
         }
 
-        try {
-          // Creating the new BlobCoreMetadata as a modified clone of the existing one
-          BlobCoreMetadataBuilder bcmBuilder = new BlobCoreMetadataBuilder(blobMetadata,
solrServerMetadata.getGeneration());
-
-          Directory snapshotIndexDir = solrCore.getDirectoryFactory().get(solrServerMetadata.getSnapshotDirPath(),
DirectoryFactory.DirContext.DEFAULT, solrCore.getSolrConfig().indexConfig.lockType);
-          try {
+        // add the old core.metadata file to delete
+        if (!currentMetadataSuffix.equals(SharedShardMetadataController.METADATA_NODE_DEFAULT_VALUE))
{
+          // TODO This may be inefficient but we'll likely remove this when CorePushPull
is refactored to have deletion elsewhere
+          //      could be added to resolvedMetadataResult#getFilesToDelete()
+          ToFromJson<BlobCoreMetadata> converter = new ToFromJson<>();
+          String json = converter.toJson(blobMetadata);
+          int bcmSize = json.getBytes().length;
+
+          String blobCoreMetadataName = BlobStoreUtils.buildBlobStoreMetadataName(currentMetadataSuffix);
+          String coreMetadataPath = blobMetadata.getSharedBlobName() + "/" + blobCoreMetadataName;
+          // so far checksum is not used for metadata file
+          BlobCoreMetadata.BlobFileToDelete bftd = new BlobCoreMetadata.BlobFileToDelete("",
coreMetadataPath, bcmSize, BlobCoreMetadataBuilder.UNDEFINED_VALUE, System.currentTimeMillis());
+          bcmBuilder.addFileToDelete(bftd);
+        }
 
-            /*
-             * Removing from the core metadata the files that should no longer be there.
-             * 
-             * TODO
-             * This is a little confusing: This is equivalent to what we were doing in first-party

-             * where the files to delete for a push were just the files that we determined
were 
-             * missing locally but on blob (filesToPull) for a push. This operates on the
assumption
-             * that the core locally was refreshed with what was in blob before this update
(both in
-             * first party and Solr Cloud). 
-             * 
-             * SharedMetadataResolutionResult makes no distinction between what action is
being taken 
-             * (push or pull) hence the confusing method naming but leaving this for now
while we reach
-             * blob feature parity.
-             * 
-             * The deletion logic will move out of this class in the future and make this
less confusing. 
-             */
-            for (BlobCoreMetadata.BlobFile d : resolvedMetadataResult.getFilesToDelete())
{
-                bcmBuilder.removeFile(d);
-                BlobCoreMetadata.BlobFileToDelete bftd = new BlobCoreMetadata.BlobFileToDelete(d,
System.currentTimeMillis());
-                bcmBuilder.addFileToDelete(bftd);
-            }
-            
-            // add the old core.metadata file to delete
-            if (!currentMetadataSuffix.equals(SharedShardMetadataController.METADATA_NODE_DEFAULT_VALUE))
{
-              // TODO This may be inefficient but we'll likely remove this when CorePushPull
is refactored to have deletion elsewhere
-              //      could be added to resolvedMetadataResult#getFilesToDelete()
-              ToFromJson<BlobCoreMetadata> converter = new ToFromJson<>();
-              String json = converter.toJson(blobMetadata);
-              int bcmSize = json.getBytes().length;
-              
-              String blobCoreMetadataName = BlobStoreUtils.buildBlobStoreMetadataName(currentMetadataSuffix);
-              String coreMetadataPath = blobMetadata.getSharedBlobName() + "/" + blobCoreMetadataName;
-              // so far checksum is not used for metadata file
-              BlobCoreMetadata.BlobFileToDelete bftd = new BlobCoreMetadata.BlobFileToDelete("",
coreMetadataPath, bcmSize, BlobCoreMetadataBuilder.UNDEFINED_VALUE, System.currentTimeMillis());
-              bcmBuilder.addFileToDelete(bftd);
-            }
-            
-            // Directory's javadoc says: "Java's i/o APIs not used directly, but rather all
i/o is through this API"
-            // But this is untrue/totally false/misleading. SnapPuller has File all over.
-            for (CoreFileData cfd : resolvedMetadataResult.getFilesToPush()) {
-              // Sanity check that we're talking about the same file (just sanity, Solr doesn't
update files so should never be different)
-              assert cfd.getFileSize() == snapshotIndexDir.fileLength(cfd.getFileName());
-
-              String blobPath = pushFileToBlobStore(coreStorageClient, snapshotIndexDir,
cfd.getFileName(), cfd.getFileSize());
-              bcmBuilder.addFile(new BlobCoreMetadata.BlobFile(cfd.getFileName(), blobPath,
cfd.getFileSize(), cfd.getChecksum()));
-            }
-          } finally {
-            solrCore.getDirectoryFactory().release(snapshotIndexDir);
+        // When we build solrServerMetadata we requested to reserve the commit point for
some short duration. Assumption is
+        // it took less than this duration to get here (didn't do anything blocking) and
now we actually save the commit
+        // point for the (pontentially long) time it takes to push all files to the Blob
store.
 
 Review comment:
   "pontentially" spelling bug

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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


Mime
View raw message