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

mds: drop partial entry and adjust write_pos when opening PurgeQueue #14447

Merged
merged 2 commits into from Apr 20, 2017

Conversation

Projects
None yet
2 participants
@ukernel
Member

ukernel commented Apr 11, 2017

At tail journal, there can be partial written entry. Before appending
new entries to the journal, we need to drop any partial written entry
and adjust write_pos. For mds log, partial written entry is detected
and dropped when replaying the journal.

For PurgeQueue journal, we don't replay the whole journal when MDS
starts. Before appending new entry to the journal, we need to drop
any partial written entry and adjust write_pos.

Fixes: http://tracker.ceph.com/issues/19450
Signed-off-by: "Yan, Zheng" zyan@redhat.com

@ukernel ukernel requested a review from jcsp Apr 11, 2017

@ukernel ukernel changed the title from mds: don't probe end of purge queue log to [DNM ]mds: don't probe end of purge queue log Apr 11, 2017

@ukernel ukernel changed the title from [DNM ]mds: don't probe end of purge queue log to mds: drop partial entry and adjust write_pos when opening PurgeQueue Apr 12, 2017

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 15, 2017

I think we could probably just avoid all partial entries by making the journaler_allow_split_entries setting configurable on a per-instance basis, and then setting it on the PurgeQueue Journaler instance. If we are always using a non-striped layout for the purge queue journal, then we should never see a split entry, right?

@ukernel

This comment has been minimized.

Member

ukernel commented Apr 17, 2017

I think 'journaler_allow_split_entries==false' is broken. JournalStream::read() don't understand the zero padding

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 17, 2017

Oh, that's annoying :-/

ukernel added some commits Apr 12, 2017

osdc/Journaler: make header write_pos align to boundary of flushed entry
This can speed up the process that detects and drops partial written
entry in the log tail.

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
mds: drop partial entry and adjust write_pos when opening PurgeQueue
At tail journal, there can be partial written entry. Before appending
new entries to the journal, we need to drop any partial written entry
and adjust write_pos. For mds log, partial written entry is detected
and dropped when replaying the journal.

For PurgeQueue journal, we don't replay the whole journal when MDS
starts. Before appending new entry to the journal, we need to drop
any partial written entry and adjust write_pos.

Previous patch makes the journal header write_pos align to boundary
of fully flushed entry. We can start finding partial written entry
from the journal header write_pos. It should be fast even when the
purge queue is very large.

Fixes: http://tracker.ceph.com/issues/19450
Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 19, 2017

The PurgeQueue change looks correct to me, but I'm not sure the Journaler optimisation is really necessary (and it adds another variable to an already slightly complicated class).

Since we already write out the header at journaler_write_head_interval, I'm not sure what the concern is about the difference between the header write_pos and the probed write_pos getting too large?

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 19, 2017

retest this please (jenkins)

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 19, 2017

This passes the fs suite. Just need to resolve whether the journaler change is really needed?

@ukernel

This comment has been minimized.

Member

ukernel commented Apr 20, 2017

without the journaler change, we need to write code that scans the log, finds start position of valid log entry.

@jcsp

jcsp approved these changes Apr 20, 2017

@jcsp jcsp merged commit f854e37 into ceph:master Apr 20, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@ukernel ukernel deleted the ukernel:wip-19450 branch May 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment