ebehrendt commented on a change in pull request #1195: SOLR-13101: Log accurate 
file counts for Push and Pull and CorePushPull
URL: https://github.com/apache/lucene-solr/pull/1195#discussion_r369260884
 
 

 ##########
 File path: 
solr/core/src/java/org/apache/solr/store/blob/metadata/CorePushPull.java
 ##########
 @@ -419,45 +433,51 @@ protected String pushFileToBlobStore(CoreStorageClient 
blob, Directory dir, Stri
 
     /**
      * Logs soblb line for push or pull action 
-     * TODO: This is for callers of this method.
-     * fileAffected and bytesTransferred represent correct values only in case 
of success
-     * In case of failure(partial processing) we are not accurate.
-     * Do we want to change that? If yes, then in case of pull is downloading 
of files locally to temp folder is considered
-     * transfer or moving from temp dir to final destination. One option could 
be to just make them -1 in case of failure.
      */
-    private void logBlobAction(String action, long filesAffected, long 
bytesTransferred, long requestQueuedTimeMs, int attempt, long startTimeMs) 
throws Exception {
+    private void logBlobAction(String action, long expectedFilesAffected, long 
actualFilesAffected, long bytesTransferred, boolean isSuccessful,
+        long requestQueuedTimeMs, int attempt, long startTimeMs) throws 
Exception {
       long now = System.nanoTime();
       long runTime = now - startTimeMs;
       long startLatency = now - requestQueuedTimeMs;
 
       String message = String.format(Locale.ROOT,
             "PushPullData=[%s] action=%s storageProvider=%s bucketRegion=%s 
bucketName=%s "
-              + "runTime=%s startLatency=%s bytesTransferred=%s attempt=%s 
filesAffected=%s localGeneration=%s blobGeneration=%s ",
+              + "runTime=%s startLatency=%s bytesTransferred=%s attempt=%s 
expectedFilesAffected=%s actualFilesAffected=%s isSuccessful=%s "
+              + "localGeneration=%s blobGeneration=%s ",
           pushPullData.toString(), action, 
coreStorageClient.getStorageProvider().name(), 
coreStorageClient.getBucketRegion(),
-          coreStorageClient.getBucketName(), runTime, startLatency, 
bytesTransferred, attempt, filesAffected,
-          solrServerMetadata.getGeneration(), blobMetadata.getGeneration());
+          coreStorageClient.getBucketName(), runTime, startLatency, 
bytesTransferred, attempt, expectedFilesAffected, actualFilesAffected,
+          isSuccessful, solrServerMetadata.getGeneration(), 
blobMetadata.getGeneration());
       log.info(message);
     }
 
     /**
      * Downloads files from the Blob store 
      * @param destDir (temporary) directory into which files should be 
downloaded.
      * @param filesToDownload blob files to be downloaded
+     * @return number of files downloaded and number of bytes transferred 
successfully
      */
     @VisibleForTesting
-    protected void downloadFilesFromBlob(Directory destDir, Collection<? 
extends BlobFile> filesToDownload) throws Exception {
+    protected Pair<Long, Long> downloadFilesFromBlob(Directory destDir, 
Collection<? extends BlobFile> filesToDownload) throws Exception {
       // Synchronously download all Blob blobs (remember we're running on an 
async thread, so no need to be async twice unless
       // we eventually want to parallelize downloads of multiple blobs, but 
for the PoC we don't :)
+      long filesDownloaded = 0;
+      long bytesTransferred = 0;
       for (BlobFile bf: filesToDownload) {
         log.info("About to create " + bf.getSolrFileName() + " for core " + 
pushPullData.getCoreName() +
             " from index on blob " + pushPullData.getSharedStoreName());
-        IndexOutput io = destDir.createOutput(bf.getSolrFileName(), 
DirectoryFactory.IOCONTEXT_NO_CACHE);
 
-        try (OutputStream outStream = new IndexOutputStream(io);
-          InputStream bis = coreStorageClient.pullStream(bf.getBlobName())) {
-          IOUtils.copy(bis, outStream);
-        }
+          try (IndexOutput io = destDir.createOutput(bf.getSolrFileName(), 
DirectoryFactory.IOCONTEXT_NO_CACHE);
+              OutputStream outStream = new IndexOutputStream(io);
+            InputStream bis = coreStorageClient.pullStream(bf.getBlobName())) {
+            IOUtils.copy(bis, outStream);
+            
+            filesDownloaded++;
+            bytesTransferred += bf.getFileSize();
+          } catch (Exception e) {
+            return new Pair<>(filesDownloaded, bytesTransferred);
 
 Review comment:
   Anyone have a recommendation on how to do this more gracefully than return 
the same values in and outside of the catch? I want to return these values 
whether exception is thrown or not, but cannot return from a finally.

----------------------------------------------------------------
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