hangc0276 commented on a change in pull request #2742:
URL: https://github.com/apache/bookkeeper/pull/2742#discussion_r689210122



##########
File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileChannelProvider.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.bookie;
+
+import java.io.File;
+import java.io.IOException;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+
+/**
+ * An interface of the FileChannelProvider.
+ */
+public interface FileChannelProvider {
+    /**
+     *
+     * @param providerClassName Provided class name for file channel.
+     * @return
+     * @throws IOException
+     */
+    static FileChannelProvider newProvider(String providerClassName) throws 
IOException {
+        try {
+            Class<?> providerClass = Class.forName(providerClassName);
+            Object obj = providerClass.getDeclaredConstructor().newInstance();

Review comment:
       Why use `getDeclaredConstructor`?

##########
File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java
##########
@@ -85,37 +86,41 @@
     // The position of the file channel's last drop position
     private long lastDropPosition = 0L;
 
+    final ServerConfiguration configuration;
+
     // Mostly used by tests
     JournalChannel(File journalDirectory, long logId) throws IOException {
-        this(journalDirectory, logId, 4 * 1024 * 1024, 65536, START_OF_FILE);
+        this(journalDirectory, logId, 4 * 1024 * 1024, 65536, START_OF_FILE, 
new ServerConfiguration());

Review comment:
       just define `MB = 1024 * 1024` maybe better.

##########
File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java
##########
@@ -143,27 +148,32 @@
     private JournalChannel(File journalDirectory, long logId,
                            long preAllocSize, int writeBufferSize, int 
journalAlignSize,
                            long position, boolean fRemoveFromPageCache,
-                           int formatVersionToWrite, 
Journal.BufferedChannelBuilder bcBuilder) throws IOException {
+                           int formatVersionToWrite, 
Journal.BufferedChannelBuilder bcBuilder,
+                           ServerConfiguration configuration) throws 
IOException {
         this.journalAlignSize = journalAlignSize;
         this.zeros = ByteBuffer.allocate(journalAlignSize);
         this.preAllocSize = preAllocSize - preAllocSize % journalAlignSize;
         this.fRemoveFromPageCache = fRemoveFromPageCache;
+        this.configuration = configuration;
+
         File fn = new File(journalDirectory, Long.toHexString(logId) + ".txn");
+        FileChannelProvider provider;
+        provider = 
FileChannelProvider.newProvider(configuration.getJournalChannelProvider());

Review comment:
       ```Java
   FileChannelProvider provider;
   provider = 
FileChannelProvider.newProvider(configuration.getJournalChannelProvider());
   ```
   to 
   `FileChannelProvider provider = 
FileChannelProvider.newProvider(configuration.getJournalChannelProvider());`

##########
File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileChannelProvider.java
##########
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.bookie;
+
+import java.io.File;
+import java.io.IOException;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+
+/**
+ * An interface of the FileChannelProvider.
+ */
+public interface FileChannelProvider {
+    /**
+     *
+     * @param providerClassName Provided class name for file channel.
+     * @return

Review comment:
       Please add docs for the return type. The same as other interfaces.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to