You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/12/01 08:09:04 UTC
[GitHub] [bookkeeper] lordcheng10 opened a new pull request, #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
lordcheng10 opened a new pull request, #3680:
URL: https://github.com/apache/bookkeeper/pull/3680
### Motivation
When building the EntryLogIdsImpl object, the ledgerDirsManager variable is used. When the EntryLogIdsImpl object is used to generate an id, all ledger directories will be scanned in the findLargestGap method. for example:
We configured three ledger directories:
/data1/bk-data1
/data1/bk-data2
/data1/bk-data3
The findLargestGap method scans these three data directories to generate ids, but actually only needs to scan one data directory corresponding to the EntryLogIdsImpl object:
https://github.com/apache/bookkeeper/blob/01232c94c91759a175345e8c055951e5cc6091dd/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java#L225-L236
### Changes
When building the EntryLogIdsImpl object, use the variable ldm instead of the variable ledgerDirsManager:
```
lDirs[0] = ledgerDir.getParentFile();
LedgerDirsManager ldm = new LedgerDirsManager(conf, lDirs, ledgerDirsManager.getDiskChecker(),
NullStatsLogger.INSTANCE);
```
```
entrylogger = new DirectEntryLogger(ledgerDir, new EntryLogIdsImpl(ldm, slog),
new NativeIOImpl(),
allocator, entryLoggerWriteExecutor, entryLoggerFlushExecutor,
conf.getEntryLogSizeLimit(),
conf.getNettyMaxFrameSizeBytes() - 500,
perDirectoryTotalWriteBufferSize,
perDirectoryTotalReadBufferSize,
readBufferSize,
numReadThreads,
maxFdCacheTimeSeconds,
slog, statsLogger);
```
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1345753326
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on a diff in pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on code in PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#discussion_r1053906132
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/TestEntryLogIds.java:
##########
@@ -127,6 +128,52 @@ public void testNoStompingDirectStartsFirst() throws Exception {
highestSoFar = logId4;
}
+ @Test
+ public void testIdGenerator() throws Exception {
+ File base = tmpDirs.createNew("entryLogIds", "ledgers");
+ File ledgerDir1 = new File(base, "l1");
+ File ledgerDir2 = new File(base, "l2");
+ File ledgerDir3 = new File(base, "l3");
+ File ledgerDir4 = new File(base, "l4");
+ ledgerDir1.mkdir();
+ ledgerDir2.mkdir();
+ ledgerDir3.mkdir();
+ ledgerDir4.mkdir();
+
+ //case 1: use root ledgerDirsManager
+ LedgerDirsManager ledgerDirsManager = newDirsManager(ledgerDir1, ledgerDir2);
+ EntryLogIds ids1 = new EntryLogIdsImpl(ledgerDirsManager, slog);
+ for (int i = 0; i < 10; i++) {
+ int logId = ids1.nextId();
+ File log1 = new File(ledgerDir1 + "/current", logId + ".log");
Review Comment:
@hangc0276
EntryLogIds generates the next id based on the existing file ids in the directory. In order to simulate the impact of multiple directories on the id, some temporary files need to be created here:
<img width="741" alt="image" src="https://user-images.githubusercontent.com/19296967/208802582-3854ab68-288e-4e6f-ac50-df4852733dda.png">
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1350325507
ping @hangc0276 @eolivelli @StevenLuMT
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1345558969
> Would you please add a test to cover it?
Fixed,PTAL,thanks! @hangc0276
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1333596570
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1355948906
Ping @hangc0276
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1345614386
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1348681596
> Would you please add a test to cover it?
Fixed,PTAL,thanks! @hangc0276
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 merged pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
hangc0276 merged PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1333374772
@hangc0276 @StevenLuMT PTAL,thanks!
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1336711467
> Would you please add a test to cover it?
OK , I will fix
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1352693191
ping @hangc0276 PTAL,thanks!
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#issuecomment-1359146421
ping PTAL,thanks! @hangc0276
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on a diff in pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on code in PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#discussion_r1053906132
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/TestEntryLogIds.java:
##########
@@ -127,6 +128,52 @@ public void testNoStompingDirectStartsFirst() throws Exception {
highestSoFar = logId4;
}
+ @Test
+ public void testIdGenerator() throws Exception {
+ File base = tmpDirs.createNew("entryLogIds", "ledgers");
+ File ledgerDir1 = new File(base, "l1");
+ File ledgerDir2 = new File(base, "l2");
+ File ledgerDir3 = new File(base, "l3");
+ File ledgerDir4 = new File(base, "l4");
+ ledgerDir1.mkdir();
+ ledgerDir2.mkdir();
+ ledgerDir3.mkdir();
+ ledgerDir4.mkdir();
+
+ //case 1: use root ledgerDirsManager
+ LedgerDirsManager ledgerDirsManager = newDirsManager(ledgerDir1, ledgerDir2);
+ EntryLogIds ids1 = new EntryLogIdsImpl(ledgerDirsManager, slog);
+ for (int i = 0; i < 10; i++) {
+ int logId = ids1.nextId();
+ File log1 = new File(ledgerDir1 + "/current", logId + ".log");
Review Comment:
EntryLogIds generates the next id based on the existing file ids in the directory. In order to simulate the impact of multiple directories on the id, some temporary files need to be created here:
<img width="741" alt="image" src="https://user-images.githubusercontent.com/19296967/208802582-3854ab68-288e-4e6f-ac50-df4852733dda.png">
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3680: [fix][server] When dbStorage_directIOEntryLogger=true, EntryLogIdsImpl only takes effect for a single ledger directory
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on code in PR #3680:
URL: https://github.com/apache/bookkeeper/pull/3680#discussion_r1053499874
##########
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/TestEntryLogIds.java:
##########
@@ -127,6 +128,52 @@ public void testNoStompingDirectStartsFirst() throws Exception {
highestSoFar = logId4;
}
+ @Test
+ public void testIdGenerator() throws Exception {
+ File base = tmpDirs.createNew("entryLogIds", "ledgers");
+ File ledgerDir1 = new File(base, "l1");
+ File ledgerDir2 = new File(base, "l2");
+ File ledgerDir3 = new File(base, "l3");
+ File ledgerDir4 = new File(base, "l4");
+ ledgerDir1.mkdir();
+ ledgerDir2.mkdir();
+ ledgerDir3.mkdir();
+ ledgerDir4.mkdir();
+
+ //case 1: use root ledgerDirsManager
+ LedgerDirsManager ledgerDirsManager = newDirsManager(ledgerDir1, ledgerDir2);
+ EntryLogIds ids1 = new EntryLogIdsImpl(ledgerDirsManager, slog);
+ for (int i = 0; i < 10; i++) {
+ int logId = ids1.nextId();
+ File log1 = new File(ledgerDir1 + "/current", logId + ".log");
Review Comment:
We just need to check `logId`, and do not need to create new files.
--
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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org