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