You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by "AuroraTwinkle (via GitHub)" <gi...@apache.org> on 2024/07/19 02:46:35 UTC
[PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
AuroraTwinkle opened a new pull request, #4466:
URL: https://github.com/apache/bookkeeper/pull/4466
Descriptions of the changes in this PR:
<!-- Either this PR fixes an issue, -->
Fix erros of rocksdb default config
### Motivation
The default rocksdb config has some errors and need fix.
### Changes
fix error rocksdb default config for CFOptions.
--
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
Re: [PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
Posted by "StevenLuMT (via GitHub)" <gi...@apache.org>.
StevenLuMT merged PR #4466:
URL: https://github.com/apache/bookkeeper/pull/4466
--
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
Re: [PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
Posted by "AuroraTwinkle (via GitHub)" <gi...@apache.org>.
AuroraTwinkle commented on PR #4466:
URL: https://github.com/apache/bookkeeper/pull/4466#issuecomment-2238322421
> The change broke KeyValueStorageRocksDBTest:
>
> ```
> Error: org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile
> Error: Run 1: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
> Error: Run 2: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
> Error: Run 3: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
> ```
Thanks，I will fix all failed Testcases.
--
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
Re: [PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #4466:
URL: https://github.com/apache/bookkeeper/pull/4466#discussion_r1717966879
##########
conf/entry_location_rocksdb.conf.default:
##########
@@ -26,7 +30,7 @@
# set by jni: options.setLogFileTimeToRoll
log_file_time_to_roll=86400
# set by jni: options.setMaxBackgroundJobs or options.setIncreaseParallelism
- max_background_jobs=2
+ max_background_jobs=32
Review Comment:
What's the rationale in making this change in this PR?
--
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
Re: [PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
Posted by "AuroraTwinkle (via GitHub)" <gi...@apache.org>.
AuroraTwinkle commented on code in PR #4466:
URL: https://github.com/apache/bookkeeper/pull/4466#discussion_r1718056226
##########
conf/entry_location_rocksdb.conf.default:
##########
@@ -26,7 +30,7 @@
# set by jni: options.setLogFileTimeToRoll
log_file_time_to_roll=86400
# set by jni: options.setMaxBackgroundJobs or options.setIncreaseParallelism
- max_background_jobs=2
+ max_background_jobs=32
Review Comment:
> What's the rationale in making this change in this PR?
Hi Lari:
The previous default value for **max_background_jobs** was written incorrectly and was not consistent with the default value in the code, so this PR corrected it. Refer to: https://github.com/apache/bookkeeper/blob/999cd0f2ab14404be4d6c24e388456dbe56bb1a8/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java#L213
--
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
Re: [PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
Posted by "dlg99 (via GitHub)" <gi...@apache.org>.
dlg99 commented on PR #4466:
URL: https://github.com/apache/bookkeeper/pull/4466#issuecomment-2238172662
The change broke KeyValueStorageRocksDBTest:
```
Error: org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile
Error: Run 1: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
Error: Run 2: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
Error: Run 3: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
```
--
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
Re: [PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
Posted by "AuroraTwinkle (via GitHub)" <gi...@apache.org>.
AuroraTwinkle commented on PR #4466:
URL: https://github.com/apache/bookkeeper/pull/4466#issuecomment-2238904475
> The change broke KeyValueStorageRocksDBTest:
>
> ```
> Error: org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile
> Error: Run 1: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
> Error: Run 2: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
> Error: Run 3: KeyValueStorageRocksDBTest.testRocksDBInitiateWithConfigurationFile:74 expected:<1> but was:<30>
> ```
I have fixed it.
--
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
Re: [PR] fix[rocksdb]: fix error rocksdb default config for CFOptions [bookkeeper]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #4466:
URL: https://github.com/apache/bookkeeper/pull/4466#discussion_r1718075577
##########
conf/entry_location_rocksdb.conf.default:
##########
@@ -26,7 +30,7 @@
# set by jni: options.setLogFileTimeToRoll
log_file_time_to_roll=86400
# set by jni: options.setMaxBackgroundJobs or options.setIncreaseParallelism
- max_background_jobs=2
+ max_background_jobs=32
Review Comment:
Thanks for explaining @AuroraTwinkle
--
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