Skip to content

411: prevent flush entry followed delete operations - #418

Merged
Yuval-Ariel merged 1 commit into
mainfrom
411-prevent-flush-single-delete-entry
Jun 12, 2023
Merged

411: prevent flush entry followed delete operations#418
Yuval-Ariel merged 1 commit into
mainfrom
411-prevent-flush-single-delete-entry

Conversation

@ayulas

@ayulas ayulas commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

No description provided.

@ayulas
ayulas requested a review from hilikspdb March 7, 2023 10:23
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from cf66057 to 8c590cd Compare March 7, 2023 10:25
@erez-speedb erez-speedb linked an issue Mar 7, 2023 that may be closed by this pull request
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 8c590cd to ddc898d Compare March 7, 2023 11:19
@Guyme Guyme linked an issue Mar 7, 2023 that may be closed by this pull request
Comment thread db/memtable.cc Outdated
Comment thread include/rocksdb/options.h Outdated
Comment thread options/db_options.cc Outdated
Comment thread include/rocksdb/options.h Outdated
@ayulas
ayulas requested review from mrambacher and removed request for mrambacher March 7, 2023 23:18
@Guyme
Guyme removed the request for review from hilikspdb March 8, 2023 14:21
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from ddc898d to 9cb3b00 Compare March 20, 2023 11:50
@ayulas
ayulas requested review from hilikspdb and mrambacher March 20, 2023 11:50
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 9cb3b00 to eaf3fba Compare March 29, 2023 03:53
@ayulas ayulas changed the title 411: prevent flush entry that has a followed single delete 411: prevent flush entry followed delete operations Mar 29, 2023
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch 2 times, most recently from 1eb75cb to 670db8a Compare March 29, 2023 08:02
hilikspdb
hilikspdb previously approved these changes Mar 29, 2023

@mrambacher mrambacher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you have unit tests to cover these changes?

Comment thread db/memtable.cc Outdated
Comment thread include/rocksdb/options.h Outdated
Comment thread db/builder.cc Outdated
Comment thread db/builder.cc Outdated
Comment thread db/compaction/compaction_iterator.cc Outdated
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 670db8a to 6b820e8 Compare April 2, 2023 02:40
hilikspdb
hilikspdb previously approved these changes Apr 2, 2023
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 6b820e8 to 9d67e4e Compare April 2, 2023 08:26
hilikspdb
hilikspdb previously approved these changes Apr 2, 2023
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 9d67e4e to a5610d1 Compare April 2, 2023 14:35
hilikspdb
hilikspdb previously approved these changes Apr 2, 2023

@hilikspdb hilikspdb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this option should be CF option but for now I approve ....

@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from a5610d1 to cfc5575 Compare April 3, 2023 01:39
@Yuval-Ariel
Yuval-Ariel self-requested a review April 3, 2023 10:10
Yuval-Ariel
Yuval-Ariel previously approved these changes Apr 3, 2023
Comment thread options/options_helper.cc Outdated
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 5e1185a to 2139b5f Compare April 17, 2023 10:34
@ayulas

ayulas commented Apr 17, 2023

Copy link
Copy Markdown
Contributor Author

@Yuval-Ariel can you pls review the db_stress db_crash changes and approve so it will go to testing? tnx

@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 2139b5f to dde478d Compare April 17, 2023 10:39
@Yuval-Ariel

Copy link
Copy Markdown
Contributor

LGTM
btw, its very hard to see only the recent changes since you force pushed after rebasing. its easier to see the changes without force pushing and at the end we can squash and merge.

Yuval-Ariel
Yuval-Ariel previously approved these changes Apr 17, 2023
Yuval-Ariel
Yuval-Ariel previously approved these changes Apr 17, 2023
@ayulas

ayulas commented Apr 17, 2023

Copy link
Copy Markdown
Contributor Author

Black box test failed I’m in it

@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from 2ca279e to fd12a59 Compare May 22, 2023 11:28
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from fd12a59 to f74f5c4 Compare June 8, 2023 10:14
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from f74f5c4 to cb6efff Compare June 8, 2023 10:48
@Yuval-Ariel

Copy link
Copy Markdown
Contributor

need to update the history file

@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from cb6efff to b02957c Compare June 11, 2023 17:27
currently during memtable flush ,  if key has a match key in the
delete range table and this record has no snapshot related to it,
we still write it with its value to SST file.
This feature keeps only the delete record and reduce SST size for later
compaction.
@ayulas
ayulas force-pushed the 411-prevent-flush-single-delete-entry branch from b02957c to 29f68e9 Compare June 11, 2023 17:48
@ayulas

ayulas commented Jun 11, 2023

Copy link
Copy Markdown
Contributor Author

history file updated and also @Yuval-Ariel remark to me

@ayulas

ayulas commented Jun 11, 2023

Copy link
Copy Markdown
Contributor Author

@Yuval-Ariel
Yuval-Ariel merged commit 594f177 into main Jun 12, 2023
@Yuval-Ariel
Yuval-Ariel deleted the 411-prevent-flush-single-delete-entry branch June 12, 2023 06:06
@Yuval-Ariel
Yuval-Ariel restored the 411-prevent-flush-single-delete-entry branch June 14, 2023 09:38
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.

prevent flush entry followed delete operations remove single delete elements during memtable flush

4 participants