Conversation
- remote link identification becomes tricky if inode is cached.
- Try to open the link normally, if issue while opening mark as damaged
- If openned successfully, it can be possible there is damage but inode
is cached that's why it is succssful while opening. In that case take
that openned inode, and scrub ancestors recursively. If any of the ancestor
is damaged it remote link is marked as damaged.
- while scrubbing some flag is maintained in the inode,
e.g. whether scrub is backward or forward or both
- his backward scrubbing will only work in read-only scrub that means
without repair flag and mds_scrub_hard_link this ceph flag is turned on.
- A new type of damage introduced, using which multiple links point to same
inode can be identified, which was not possible previously.
2. mds_damage_log_to_file and mds_damage_log_file is used to print out damages
in a file persistently as it's not safe to keep it in memory
3. missing dirfrag can make scrub recurring, so a flag `from_scrub` is used to identify when
dirfrag fetch is from scrub function.
Fixes: https://tracker.ceph.com/issues/68611
Signed-off-by: Md Mahamudur Rahaman Sajib <mahamudur.sajib@croit.io>
| } else if (dir->get_version() == 0) { | ||
| dout(20) << __func__ << " barebones " << *dir << dendl; | ||
| dir->fetch_keys({}, gather.new_sub()); | ||
| dir->fetch_keys({}, gather.new_sub(), true); |
There was a problem hiding this comment.
This (plus all the changes in CDir.*) look like a pretty independent fix, IMO better to make a standalone commit for that.
| add_remote_link_damage(remote_link_path, remote_ino); | ||
| header->inc_scrubbed_remote_link_count(); | ||
| } | ||
| in->scrub_reset_remote_links(); |
There was a problem hiding this comment.
redundant, scrub_finished() does reset on its own.
| add_remote_link_damage(remote_link_path, remote_ino); | ||
| header->inc_scrubbed_remote_link_count(); | ||
| } | ||
| in->scrub_reset_remote_links(); |
There was a problem hiding this comment.
redundant, scrub_finished() does reset on its own.
| void CInode::scrub_add_remote_link( | ||
| std::vector<std::pair<std::string, inodeno_t>> &&remote_links) { | ||
|
|
||
| for (auto& [remote_link_path, remote_ino]: remote_links) { |
There was a problem hiding this comment.
Why not using a whole pair:
for (auto& p : remote_links) {
emplace_back(std::move(p));
}
| if (done) { | ||
| dout(20) << __func__ << " dir inode, done" << dendl; | ||
| in->set_forward_scrub(false); | ||
| dequeue(in); |
There was a problem hiding this comment.
Shouldn't we call scrub_dir_inode_final(in) here if (!remote_links().empty()) ?
| log_to_file = _log_to_file; | ||
| if (log_to_file) { | ||
| log_file_opened = open_damage_log_file(fout, log_file); | ||
| } |
There was a problem hiding this comment.
For the sake of completeness IMO better to timplement log file close as well.
| void set_log_to_file(bool _log_to_file) { | ||
| log_to_file = _log_to_file; | ||
| if (log_to_file) { | ||
| log_file_opened = open_damage_log_file(fout, log_file); |
There was a problem hiding this comment.
IMO log_file_opened better to be assigned inside open_damage_log_file (and symetric close method if any).
| remote_links.erase(df_remote_link_it); | ||
| } | ||
| } | ||
| remote_links.erase(remote_link_entry->ino); |
There was a problem hiding this comment.
Apparently either this one or above remote_links.erase() call is redundant
| if (df_remote_link_it != remote_links.end()) { | ||
| auto damage_it = df_remote_link_it->second.find(entry->path); | ||
| if (damage_it != df_remote_link_it->second.end()) { | ||
| df_remote_link_it->second.erase(entry->path); |
There was a problem hiding this comment.
Why not using damage_it for erase() call?
| starttime(mono_clock::now()), | ||
| ioc(ioc) | ||
| { | ||
| MDSRank::MDSRank(mds_rank_t whoami_, ceph::fair_mutex &mds_lock_, |
There was a problem hiding this comment.
IMO the original version was better readable...
|
@sajibreadd - this can be closed in favor of #6, right? |
3 participants
from_scrubis used to identify when dirfrag fetch is from scrub function.Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins test classic perfJenkins Job | Jenkins Job Definitionjenkins test crimson perfJenkins Job | Jenkins Job Definitionjenkins test signedJenkins Job | Jenkins Job Definitionjenkins test make checkJenkins Job | Jenkins Job Definitionjenkins test make check arm64Jenkins Job | Jenkins Job Definitionjenkins test submodulesJenkins Job | Jenkins Job Definitionjenkins test dashboardJenkins Job | Jenkins Job Definitionjenkins test dashboard cephadmJenkins Job | Jenkins Job Definitionjenkins test apiJenkins Job | Jenkins Job Definitionjenkins test docsReadTheDocs | Github Workflow Definitionjenkins test ceph-volume allJenkins Jobs | Jenkins Jobs Definitionjenkins test windowsJenkins Job | Jenkins Job Definitionjenkins test rook e2eJenkins Job | Jenkins Job DefinitionYou must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.