You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/02/14 13:22:08 UTC
[GitHub] [bookkeeper] hangc0276 opened a new pull request #3046: Avoiding call fileChannelProvider init multiple times
hangc0276 opened a new pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046
### Motivation
In current JournalChannel implementation, it create fileChannelProvider instance every time, which is unnecessary.
https://github.com/apache/bookkeeper/blob/4debbbf84db09f80ffeb4154e8e6feed134be0c0/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java#L162
### Changes
Create fileChannelProvider in Journal creation, avoiding call fileChannelProvider init multiple times.
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#issuecomment-1042782304
rerun failure checks
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on a change in pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#discussion_r806349043
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -618,6 +618,8 @@ static void writePaddingBytes(JournalChannel jc, ByteBuf paddingBuffer, int jour
final File journalDirectory;
final ServerConfiguration conf;
final ForceWriteThread forceWriteThread;
+
+ private FileChannelProvider fileChannelProvider;
Review comment:
We have set the fileChannelProvider twice in try-catch, so it can't be final
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] nicoloboschi commented on a change in pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#discussion_r805991036
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -618,6 +618,8 @@ static void writePaddingBytes(JournalChannel jc, ByteBuf paddingBuffer, int jour
final File journalDirectory;
final ServerConfiguration conf;
final ForceWriteThread forceWriteThread;
+
+ private FileChannelProvider fileChannelProvider;
Review comment:
final ?
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#issuecomment-1047399453
ping @eolivelli @nicoloboschi , Please help review this PR, thanks a lot.
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#issuecomment-1045994040
rerun failure checks
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli merged pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] nicoloboschi commented on a change in pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#discussion_r805991036
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -618,6 +618,8 @@ static void writePaddingBytes(JournalChannel jc, ByteBuf paddingBuffer, int jour
final File journalDirectory;
final ServerConfiguration conf;
final ForceWriteThread forceWriteThread;
+
+ private FileChannelProvider fileChannelProvider;
Review comment:
final ?
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on a change in pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#discussion_r808728182
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -731,6 +733,14 @@ public Journal(int journalIndex, File journalDirectory, ServerConfiguration conf
LOG.debug("Last Log Mark : {}", lastLogMark.getCurMark());
}
+ try {
+ this.fileChannelProvider = FileChannelProvider.newProvider(conf.getJournalChannelProvider());
+ } catch (IOException e) {
+ LOG.warn("Failed to initiate {}, use default fileSystem provider instead.",
+ conf.getJournalChannelProvider());
+ this.fileChannelProvider = new DefaultFileChannelProvider();
Review comment:
done
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#discussion_r806594846
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -731,6 +733,14 @@ public Journal(int journalIndex, File journalDirectory, ServerConfiguration conf
LOG.debug("Last Log Mark : {}", lastLogMark.getCurMark());
}
+ try {
+ this.fileChannelProvider = FileChannelProvider.newProvider(conf.getJournalChannelProvider());
+ } catch (IOException e) {
+ LOG.warn("Failed to initiate {}, use default fileSystem provider instead.",
+ conf.getJournalChannelProvider());
+ this.fileChannelProvider = new DefaultFileChannelProvider();
Review comment:
We should fail the bookie, otherwise the system will not work as configured by the administrator
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#discussion_r806594846
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -731,6 +733,14 @@ public Journal(int journalIndex, File journalDirectory, ServerConfiguration conf
LOG.debug("Last Log Mark : {}", lastLogMark.getCurMark());
}
+ try {
+ this.fileChannelProvider = FileChannelProvider.newProvider(conf.getJournalChannelProvider());
+ } catch (IOException e) {
+ LOG.warn("Failed to initiate {}, use default fileSystem provider instead.",
+ conf.getJournalChannelProvider());
+ this.fileChannelProvider = new DefaultFileChannelProvider();
Review comment:
We should fail the bookie, otherwise the system will not work as configured by the administrator
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on a change in pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#discussion_r806349043
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -618,6 +618,8 @@ static void writePaddingBytes(JournalChannel jc, ByteBuf paddingBuffer, int jour
final File journalDirectory;
final ServerConfiguration conf;
final ForceWriteThread forceWriteThread;
+
+ private FileChannelProvider fileChannelProvider;
Review comment:
We have set the fileChannelProvider twice in try-catch, so it can't be final
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#issuecomment-1039715133
rerun failure checks
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3046: Avoiding call fileChannelProvider init multiple times
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #3046:
URL: https://github.com/apache/bookkeeper/pull/3046#issuecomment-1039715133
rerun failure checks
--
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-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org