dsmiley commented on a change in pull request #23:
URL: https://github.com/apache/solr/pull/23#discussion_r601012351



##########
File path: solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
##########
@@ -287,6 +293,51 @@ public void downloadCollectionProperties(String 
collectionName) throws IOExcepti
     }
   }
 
+  private void downloadFromZK(ConfigSetService configSetService, String 
configName, URI dir) throws IOException {
+    List<String> files = configSetService.listFilesInConfig(configName);

Review comment:
       I'm guessing listFilesInConfig does only one level?  I see you have it 
overloaded that takes "file".  This feels a bit awkward to me... can't we just 
list all files in the configSet?  The code here looks like it might not work if 
there are multiple intermediate levels (since there is no recursion), but I 
could be wrong.

##########
File path: solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
##########
@@ -287,6 +293,51 @@ public void downloadCollectionProperties(String 
collectionName) throws IOExcepti
     }
   }
 
+  private void downloadFromZK(ConfigSetService configSetService, String 
configName, URI dir) throws IOException {

Review comment:
       Instead of "ZK" in the name here, let's say "Solr".  Same for upload.

##########
File path: solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
##########
@@ -287,6 +293,51 @@ public void downloadCollectionProperties(String 
collectionName) throws IOExcepti
     }
   }
 
+  private void downloadFromZK(ConfigSetService configSetService, String 
configName, URI dir) throws IOException {
+    List<String> files = configSetService.listFilesInConfig(configName);
+    for (String file : files) {
+      List<String> children = configSetService.listFilesInConfig(configName, 
file);
+      if (children.size() == 0) {
+        log.debug("Writing file {}", file);
+        byte[] data = configSetService.getFileFromConfig(configName, file);
+        try (OutputStream os = repository.createOutput(repository.resolve(dir, 
file))) {
+          os.write(data);
+        }
+      } else {
+        URI uri = repository.resolve(dir, file);
+        if (!repository.exists(uri)) {
+          repository.createDirectory(uri);
+        }
+        downloadFromZK(configSetService, configName, repository.resolve(dir, 
file));
+      }
+    }
+  }
+
+  private void uploadToZk(ConfigSetService configSetService, URI sourceDir, 
String configName) throws IOException {
+    for (String file : repository.listAll(sourceDir)) {
+      URI path = repository.resolve(sourceDir, file);
+      BackupRepository.PathType t = repository.getPathType(path);
+      switch (t) {
+        case FILE: {
+          try (IndexInput is = repository.openInput(sourceDir, file, 
IOContext.DEFAULT)) {
+            byte[] arr = new byte[(int) is.length()]; // probably ok since the 
config file should be small.

Review comment:
       Yuck but it seems the code was this way already.

##########
File path: solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
##########
@@ -287,6 +293,51 @@ public void downloadCollectionProperties(String 
collectionName) throws IOExcepti
     }
   }
 
+  private void downloadFromZK(ConfigSetService configSetService, String 
configName, URI dir) throws IOException {
+    List<String> files = configSetService.listFilesInConfig(configName);
+    for (String file : files) {
+      List<String> children = configSetService.listFilesInConfig(configName, 
file);
+      if (children.size() == 0) {
+        log.debug("Writing file {}", file);
+        byte[] data = configSetService.getFileFromConfig(configName, file);
+        try (OutputStream os = repository.createOutput(repository.resolve(dir, 
file))) {
+          os.write(data);
+        }
+      } else {
+        URI uri = repository.resolve(dir, file);
+        if (!repository.exists(uri)) {
+          repository.createDirectory(uri);
+        }
+        downloadFromZK(configSetService, configName, repository.resolve(dir, 
file));
+      }
+    }
+  }
+
+  private void uploadToZk(ConfigSetService configSetService, URI sourceDir, 
String configName) throws IOException {
+    for (String file : repository.listAll(sourceDir)) {
+      URI path = repository.resolve(sourceDir, file);
+      BackupRepository.PathType t = repository.getPathType(path);
+      switch (t) {
+        case FILE: {
+          try (IndexInput is = repository.openInput(sourceDir, file, 
IOContext.DEFAULT)) {
+            byte[] arr = new byte[(int) is.length()]; // probably ok since the 
config file should be small.

Review comment:
       BTW @bruno-roustant , this is another place where that InputStream 
wrapper around an IndexInput could be useful in order to use common APIs that 
work with InputStreams.  I'm reminded of this from your BlobDirectory work.




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


Reply via email to