dsmiley commented on code in PR #105:
URL: https://github.com/apache/solr-sandbox/pull/105#discussion_r1594502634


##########
encryption/src/main/java/org/apache/solr/encryption/EncryptionUpdateLog.java:
##########
@@ -137,27 +151,45 @@ public synchronized boolean 
areAllLogsEncryptedWithKeyId(String keyId, SegmentIn
     return true;
   }
 
-  protected void encryptLog(EncryptionTransactionLog log) throws IOException {
+  private List<TransactionLog> getAllLogs() {
+    List<TransactionLog> allLogs = new ArrayList<>(logs.size() + 3);
+    if (tlog != null) {
+      allLogs.add(tlog);
+    }
+    if (prevTlog != null) {
+      allLogs.add(prevTlog);
+    }
+    if (bufferTlog != null) {
+      allLogs.add(bufferTlog);
+    }
+    allLogs.addAll(logs);
+    return allLogs;
+  }
+
+  /**
+   * Encrypts a transaction log if it is not encrypted with the latest 
encryption key.
+   * The transaction log is toggled in readonly mode to ensure it is not 
written anymore.
+   */
+  protected void encryptLog(EncryptionTransactionLog log, String activeKeyRef, 
EncryptionDirectory directory)
+          throws IOException {
+    assert log.refCount() <= 1;
     if (Files.size(log.path()) > 0) {
-      EncryptionDirectory directory = directorySupplier.get();
-      try {
-        directory.forceReadCommitUserData();
-        try (FileChannel inputChannel = FileChannel.open(log.path(), 
StandardOpenOption.READ)) {
-          String inputKeyRef = readEncryptionHeader(inputChannel, 
ByteBuffer.allocate(4));
-          String activeKeyRef = 
getActiveKeyRefFromCommit(directory.getLatestCommitData().data);
-          if (!Objects.equals(inputKeyRef, activeKeyRef)) {
-            Path newLog = log.path().resolveSibling(log.path().getFileName() + 
".enc");
-            try (OutputStream outputStream = new 
FileOutputStream(newLog.toFile())) {
-              reencrypt(inputChannel, inputKeyRef, outputStream, activeKeyRef, 
directory);
-            }
-            Path backupLog = 
log.path().resolveSibling(log.path().getFileName() + ".bak");
-            Files.move(log.path(), backupLog);
-            Files.move(newLog, log.path());
-            Files.delete(backupLog);
+      try (FileChannel inputChannel = FileChannel.open(log.path(), 
StandardOpenOption.READ)) {
+        String inputKeyRef = readEncryptionHeader(inputChannel, 
ByteBuffer.allocate(4));
+        if (!Objects.equals(inputKeyRef, activeKeyRef)) {
+          Path newLogPath = log.path().resolveSibling(log.path().getFileName() 
+ ".enc");
+          try (OutputStream outputStream = new 
FileOutputStream(newLogPath.toFile())) {

Review Comment:
   So this PR uses NIO almost completely except for this line.  Try 
`Files.newOutputStream` instead of referencing FileOutputStream and without the 
`Path.toFile` that we should try and avoid.



##########
encryption/src/main/java/org/apache/solr/encryption/EncryptionTransactionLog.java:
##########
@@ -156,7 +169,7 @@ private static int readBEInt(ReadableByteChannel channel, 
ByteBuffer readBuffer)
   }
 
   /** Supplies and releases {@link EncryptionDirectory}. */
-  protected interface EncryptionDirectorySupplier {
+  public interface EncryptionDirectorySupplier {

Review Comment:
   Note that Solr uses `RefCounted` for this scenario in a number of cases.  
This could be another?



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

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to