Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable speedb features rebase #543

Merged
merged 1 commit into from
Aug 14, 2023
Merged

Conversation

RoyBenMoshe
Copy link
Contributor

No description provided.

@udi-speedb
Copy link
Contributor

Running make_format on the branch still has formatting issues

include/rocksdb/options.h Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
@udi-speedb
Copy link
Contributor

@RoyBenMoshe - An initial batch of comments. Let's discuss

examples/Makefile Outdated Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
tools/db_bench_tool.cc Outdated Show resolved Hide resolved
options/options.cc Show resolved Hide resolved
options/options.cc Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
@udi-speedb udi-speedb self-requested a review June 10, 2023 11:42
include/rocksdb/options.h Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Outdated Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
options/options.cc Show resolved Hide resolved
options/options.cc Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
@udi-speedb
Copy link
Contributor

@RoyBenMoshe Another batch of comments. I am not done yet, but will wait with the rest until you have addressed the comments in this batch. Thanks

@udi-speedb udi-speedb self-requested a review June 11, 2023 15:50
examples/enable_speedb_features_example.cc Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
options/options.cc Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
options/options_test.cc Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
@udi-speedb
Copy link
Contributor

Please update the HISTORY.md as part of the PR as well.

include/rocksdb/options.h Show resolved Hide resolved
// enable the spdb features
// please note that a call to enable speedb options in the level of cf should
// follow
DBOptions* EnableSpeedbFeaturesDB(SpeedbSharedOptions& shared_options);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above (the cf part)

include/rocksdb/options.h Outdated Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
options/options_test.cc Show resolved Hide resolved
tools/db_bench_tool.cc Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
examples/Makefile Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
examples/enable_speedb_features_example.cc Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
include/rocksdb/options.h Outdated Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
options/options.cc Outdated Show resolved Hide resolved
options/options_test.cc Outdated Show resolved Hide resolved
@udi-speedb udi-speedb self-requested a review June 14, 2023 02:53
@RoyBenMoshe RoyBenMoshe requested a review from ayulas July 11, 2023 16:24
options/options.cc Show resolved Hide resolved
options/options.cc Show resolved Hide resolved
options/options.cc Show resolved Hide resolved
options/options_test.cc Show resolved Hide resolved
@RoyBenMoshe RoyBenMoshe force-pushed the enable_speedb_features_rebase branch from 6975047 to 46f8553 Compare July 12, 2023 16:59
@Guyme Guyme linked an issue Jul 24, 2023 that may be closed by this pull request
@Yuval-Ariel
Copy link
Contributor

on hold until #610 is pushed

@Yuval-Ariel
Copy link
Contributor

This cannot be merged until the requested changes from @mrambacher are addressed.

@@ -299,8 +303,6 @@ DEFINE_string(
"\twaitforcompaction - pause until compaction is (probably) done\n"
"\tflush - flush the memtable\n"
"\tstats -- Print DB stats\n"
"\ttable-readers-mem -- Print table readers memory. excluding memory "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it is related?

}

void SharedOptions::IncreaseWriteBufferSize(size_t increase_by) {
if (write_buffer_manager->buffer_size() == 1 && increase_by > 1) {
write_buffer_manager->SetBufferSize(increase_by);
if (total_ram_size_bytes_ / 4 > increase_by) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using 4 is not the right way . pls defined and. use the define

}

void SharedOptions::IncreaseWriteBufferSize(size_t increase_by) {
if (write_buffer_manager->buffer_size() == 1 && increase_by > 1) {
write_buffer_manager->SetBufferSize(increase_by);
if (total_ram_size_bytes_ / 4 > increase_by) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls calc the increase size on a local parameter, and just call SetBufferSize once with the correct value

@autogit-speedb autogit-speedb force-pushed the enable_speedb_features_rebase branch 2 times, most recently from 57a129b to 98fbb56 Compare August 13, 2023 13:25
@Yuval-Ariel Yuval-Ariel merged commit 2075487 into main Aug 14, 2023
11 checks passed
@Yuval-Ariel Yuval-Ariel deleted the enable_speedb_features_rebase branch August 14, 2023 04:38
udi-speedb pushed a commit that referenced this pull request Nov 22, 2023
udi-speedb pushed a commit that referenced this pull request Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tuning Function - optimize db for Speedb
6 participants