mds: scrub fix

[sajibreadd-croit]

1. remote link damage identification with reverse parent scrubbing
   • 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

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 x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

• Tracker (select at least one)
  • References tracker ticket
  • Very recent bug; references commit where it was introduced
  • New feature (ticket optional)
  • Doc update (no ticket needed)
  • Code cleanup (no ticket needed)
• Component impact
  • Affects Dashboard, opened tracker ticket
  • Affects Orchestrator, opened tracker ticket
  • No impact that needs to be tracked
• Documentation (select at least one)
  • Updates relevant documentation
  • No doc update is appropriate
• Tests (select at least one)
  • Includes unit test(s)
  • Includes integration test(s)
  • Includes bug reproducer
  • No tests

Show available Jenkins commands

• jenkins test classic perf Jenkins Job | Jenkins Job Definition
• jenkins test crimson perf Jenkins Job | Jenkins Job Definition
• jenkins test signed Jenkins Job | Jenkins Job Definition
• jenkins test make check Jenkins Job | Jenkins Job Definition
• jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
• jenkins test submodules Jenkins Job | Jenkins Job Definition
• jenkins test dashboard Jenkins Job | Jenkins Job Definition
• jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
• jenkins test api Jenkins Job | Jenkins Job Definition
• jenkins test docs ReadTheDocs | Github Workflow Definition
• jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
• jenkins test windows Jenkins Job | Jenkins Job Definition
• jenkins test rook e2e Jenkins Job | Jenkins Job Definition

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

[sajibreadd-croit]

@ifed01 regarding this comment #5 (comment)

Shouldn't we call scrub_dir_inode_final(in) here if (!remote_links().empty()) ?

if (!remote_links.empty) then inside scrub_dir_inode it will call scrub_dir_inode_final(in) and handle the backward scrub there.

[ifed01]

hmm... But calling scrub_dir_inode_final() would happen before forward_scrub is cleared so it would be a no-op in regard to remote links, nor?

[ifed01]

IMO better rephrase to "force hard link scrubbing"

[ifed01]

May be bettet to check for non_empty log_file before calling open_damage_log_file() to avoid redundant errors on empy file opening attempt in the log.

[ifed01]

I don't understand why do we need to enqueue it once again here - IIUC this will effectively do nothing as remote_links are emty and in->scrub_is_in_progress() == true

[ifed01]

And if this block is removed then scrub_reset_remote_links() call above is redundant as well.

[sajibreadd-croit]

First of all scrubbing operation are not parallel in mds side, it parallalize the I/O towards osd only.

void *MDSRank::ProgressThread::entry()
{
  std::unique_lock l(mds->mds_lock);
  while (true) {
    cond.wait(l, [this] {
      return (mds->stopping ||
	      !mds->finished_queue.empty() ||
	      (!mds->waiting_for_nolaggy.empty() && !mds->beacon.is_laggy()));
    });

    if (mds->stopping) {
      break;
    }

    mds->_advance_queues();
  }

  return NULL;
}

This progress thread runs every task sequentially and under big mds_lock

I don't understand why do we need to enqueue it once again here

Let's say forward_scrub is false, in that case it will scrub backwards. Now for a specific inode it sends request to osd through in->validate_disk_state(&fin->result, fin);. This I/O is asynchronous. So in this async I/O, what if in between some directory recursed through that inode and tries to enqeue it for forward scrubbing. But as scrub is in progress (asynchronously) it won't push that inode into the queue but make forward_scrub = true here.

int ScrubStack::_enqueue(
    MDSCacheObject *obj, ScrubHeaderRef &header, bool top, bool *added,
    std::vector<std::pair<std::string, inodeno_t>> &&remote_links) {
  ceph_assert(ceph_mutex_is_locked_by_me(mdcache->mds->mds_lock));
  if (CInode *in = dynamic_cast<CInode*>(obj)) {
    if (in->scrub_is_in_progress()) {
      dout(10) << __func__ << " with {" << *in << "}" << ", already in scrubbing" << dendl;
      if (!remote_links.empty()) {
        in->scrub_add_remote_link(std::move(remote_links));
      } else {
        in->set_forward_scrub(true);
      }
      return -CEPHFS_EBUSY;
    }

[sajibreadd-croit]

You are right. We need to make scrub_is_in_progress false before _enqueue call