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

Change osd journal padding mode. #5877

Closed
wants to merge 4 commits into from

Conversation

majianpeng
Copy link
Member

Now for every entry, we pad data if need. But for most case, we fetch more entries as a system-write. So we can add padding data at the end of system-write content if need. This can reduce much padding data for small write.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
The reasons are align_bl don't need protect of write_lock and align_bl
may take long time(do  memcopy data).

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Current, because the requirement of DirectIo, the padding mode
is adding padding data for every entry.
In fact, we fetch a couple of entries as a system write as possible
as can. So we only make sure system-write content met the requirement
of DirectIO. We adding a padding entry at the end of system-write
content(for most case, this include a couple of entries).

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
@majianpeng
Copy link
Member Author

@liewegas . A while ago, i sent a PR(#5489) which want to reduce the padding data size. But it made no sense. So i modify the PR and hope you review this PR. Thanks!

@majianpeng
Copy link
Member Author

@liewegas . Ping!

@XinzeChi
Copy link
Contributor

@majianpeng , I think it is a good strategy for hdd. But In my test, the write journal thread is bottleneck in fast equipment because of the crc and rebuild logic, such as pcie ssd. You could get more detail information from ceph-devel mailist.

@XinzeChi
Copy link
Contributor

XinzeChi commented Dec 7, 2015

@majianpeng , @liewegas , I think this is make sense for some type of equipments.
So could we set two mode for filejournal. one is master branch strategy, the other is this patch?
we could add new parameter in config_opts.h and user would choose it?

Please review it #6856.

@liewegas
Copy link
Member

liewegas commented Dec 8, 2015

I'm very nervous about making this sort of change unless there is a demonstrated and significant performance improvement. Otherwise we risk introducing new bugs in critical code for little or no gain.

@XinzeChi
Copy link
Contributor

XinzeChi commented Dec 9, 2015

@liewegas , I add the benchmakr at #6856

@liewegas liewegas closed this Feb 4, 2016
@ghost ghost added the core label Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants