[HBASE-28665] WALs not marked closed when there are errors in closing WALs

• Type: Bug
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 2.5.8
• Fix Version/s: 2.7.0, 2.6.1, 2.5.10
• Component/s: wal
• Labels: pull-request-available
• Hadoop Flags: Reviewed
• Assignee: Kiran Kumar Maturi
• Reporter: Kiran Kumar Maturi
• Created: 14/Jun/24 08:22
• Updated: 12/Jul/24 04:49
• Resolved: 11/Jul/24 06:02

Description

In our production clusters we have observed that when WAL close fails It causes the the oldWAL files not marked as close and not letting them cleaned. When a WAL close fails in closeWriter it increments the error count.

Span span = Span.current();
 try {
      span.addEvent("closing writer");
      writer.close();
      span.addEvent("writer closed");
    } catch (IOException ioe) {
      int errors = closeErrorCount.incrementAndGet();
      boolean hasUnflushedEntries = isUnflushedEntries();
      if (syncCloseCall && (hasUnflushedEntries || (errors > this.closeErrorsTolerated))) {
        LOG.error("Close of WAL " + path + " failed. Cause=\"" + ioe.getMessage() + "\", errors="
          + errors + ", hasUnflushedEntries=" + hasUnflushedEntries);
        throw ioe;
      }
      LOG.warn("Riding over failed WAL close of " + path
        + "; THIS FILE WAS NOT CLOSED BUT ALL EDITS SYNCED SO SHOULD BE OK", ioe);
    }

When there are errors in closing WAL only twice doReplaceWALWriter enters this code block

if (isUnflushedEntries() || closeErrorCount.get() >= this.closeErrorsTolerated) {
          try {
            closeWriter(this.writer, oldPath, true);
          } finally {
            inflightWALClosures.remove(oldPath.getName());
          }
        }

as we don't mark them closed here like we do it here

  Writer localWriter = this.writer;
          closeExecutor.execute(() -> {
            try {
              closeWriter(localWriter, oldPath, false);
            } catch (IOException e) {
              LOG.warn("close old writer failed", e);
            } finally {
              // call this even if the above close fails, as there is no other chance we can set
              // closed to true, it will not cause big problems.
             {color:red} markClosedAndClean(oldPath);{color}
              inflightWALClosures.remove(oldPath.getName());
            }
          });

Issue Links

• links to: GitHub Pull Request #6027
• links to: GitHub Pull Request #6041