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.

Sign up for GitHub

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

Jump to bottom

WAL sync performance optimizations #200

Merged
merged 2 commits into from
Feb 7, 2022
Merged

WAL sync performance optimizations #200

merged 2 commits into from
Feb 7, 2022

Conversation

nabeelmmd
Copy link
Contributor

Signed-off-by: Nabeel M Mohamed 50154757+nabeelmmd@users.noreply.github.com

@nabeelmmd
WAL performance optimizations to support frequent syncs on a slow IO …
be5e184 
…backend

Signed-off-by: Nabeel M Mohamed <50154757+nabeelmmd@users.noreply.github.com>
@nabeelmmd nabeelmmd self-assigned this Feb 7, 2022
tristan957
tristan957 reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.c Outdated Show resolved Hide resolved
tristan957
tristan957 reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.c Outdated Show resolved Hide resolved
tristan957
tristan957 reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.h Outdated Show resolved Hide resolved
tristan957
tristan957 reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal_buffer.c Outdated Show resolved Hide resolved
@tristan957
Copy link
Collaborator

possible to have any tests?

@nabeelmmd
Copy link
Contributor Author

possible to have any tests?

I've added a test for the param changes.. The wal changes should be exercised by our current tests as it's in a common path.

@tristan957
Copy link
Collaborator

Only thing I would say is squash the commits yourself or change the default commit message before merging. GitHub has a dumb default commit message when you have multiple commits in a PR for squash and merge and strat. :)

alexttx
alexttx reviewed Feb 7, 2022
View reviewed changes
lib/kvdb/kvdb_rparams.c Outdated Show resolved Hide resolved
alexttx
alexttx reviewed Feb 7, 2022
View reviewed changes
lib/include/hse_ikvdb/wal.h Outdated Show resolved Hide resolved
@nabeelmmd
Copy link
Contributor Author

Only thing I would say is squash the commits yourself or change the default commit message before merging. GitHub has a dumb default commit message when you have multiple commits in a PR for squash and merge and strat. :)

Oh I see.. I thought it just combines all the commit messages and puts it in the merge commit? you don't like that?

alexttx
alexttx reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.c Show resolved Hide resolved
@nabeelmmd nabeelmmd force-pushed the nabeelmmd/wal-sync-optimizations branch from f1e0245 to 4d8351b Compare February 7, 2022 18:40
alexttx
alexttx approved these changes Feb 7, 2022
View reviewed changes
lib/wal/wal.c Show resolved Hide resolved
lib/wal/wal.h Show resolved Hide resolved
@nabeelmmd nabeelmmd force-pushed the nabeelmmd/wal-sync-optimizations branch from 4d8351b to 1e4de07 Compare February 7, 2022 19:15
beckerg
beckerg reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.c Outdated Show resolved Hide resolved
beckerg
beckerg reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.c Show resolved Hide resolved
beckerg
beckerg reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.c Outdated Show resolved Hide resolved
beckerg
beckerg reviewed Feb 7, 2022
View reviewed changes
lib/wal/wal.c Outdated Show resolved Hide resolved
@tristan957
Copy link
Collaborator

Only thing I would say is squash the commits yourself or change the default commit message before merging. GitHub has a dumb default commit message when you have multiple commits in a PR for squash and merge and strat. :)

Oh I see.. I thought it just combines all the commit messages and puts it in the merge commit? you don't like that?

It puts the signed-off-by multiple times and the "Address review comments" gets left in the message too.

@nabeelmmd nabeelmmd force-pushed the nabeelmmd/wal-sync-optimizations branch 2 times, most recently from 15d987a to 9c4b6c0 Compare February 7, 2022 22:02
@nabeelmmd
Address review comments
860f24b 
Signed-off-by: Nabeel M Mohamed <50154757+nabeelmmd@users.noreply.github.com>
@nabeelmmd nabeelmmd force-pushed the nabeelmmd/wal-sync-optimizations branch from 9c4b6c0 to 860f24b Compare February 7, 2022 22:23
@nabeelmmd nabeelmmd merged commit 4c0668d into master Feb 7, 2022
@nabeelmmd nabeelmmd deleted the nabeelmmd/wal-sync-optimizations branch February 7, 2022 23:09
nabeelmmd added a commit that referenced this pull request Feb 7, 2022
@nabeelmmd
WAL sync performance optimizations (NFSE-5149) (#200)
b2de6e8 
WAL performance optimizations to support frequent syncs on a slow IO backend

Signed-off-by: Nabeel M Mohamed <50154757+nabeelmmd@users.noreply.github.com>
(cherry picked from commit 4c0668d)
nabeelmmd added a commit that referenced this pull request Feb 8, 2022
@nabeelmmd
WAL sync performance optimizations (NFSE-5149) (#200)
d73d25b 
WAL performance optimizations to support frequent syncs on a slow IO backend

Signed-off-by: Nabeel M Mohamed <50154757+nabeelmmd@users.noreply.github.com>
(cherry picked from commit 4c0668d)
nabeelmmd added a commit that referenced this pull request Feb 8, 2022
@nabeelmmd
WAL sync performance optimizations (NFSE-5149) (#200) (#201)
049e790 
WAL performance optimizations to support frequent syncs on a slow IO backend

Signed-off-by: Nabeel M Mohamed <50154757+nabeelmmd@users.noreply.github.com>
(cherry picked from commit 4c0668d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tristan957 tristan957 tristan957 approved these changes

@beckerg beckerg beckerg left review comments

@alexttx alexttx alexttx approved these changes

@gsramdasi gsramdasi Awaiting requested review from gsramdasi

@jjacob512 jjacob512 Awaiting requested review from jjacob512

Assignees

@nabeelmmd nabeelmmd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nabeelmmd @tristan957 @alexttx @beckerg