zouhuajian commented on PR #8709: URL: https://github.com/apache/hadoop/pull/8709#issuecomment-5754545252
> Hi @zouhuajian , thanks for this fix. We are evaluating [HDFS-17970](https://issues.apache.org/jira/browse/HDFS-17970) for production and have two questions: > > 1. Have you deployed this patch in production? If so, have you observed any issues with EC checksum reconstruction, particularly during DataNode decommissioning or when duplicate internal-block replicas are present? > 2. Have you considered duplicate replicas being counted as independent sources during reconstruction fallback? > > We reproduced the following case on trunk `3f440d242312` plus this PR’s commit `ac4a2ce42984`, using JDK 17. > > For RS-6-3, reconstructing index `0`, the source indices are ordered as the patch allows: > > ``` > [1, 2, 3, 4, 5, 6, 1, 6] > ``` > > If opening the first index-6 reader fails, `StripedReader.initReaders()` selects `[1, 2, 3, 4, 5, 1]` and stops after six successful readers. The healthy index-6 replica at the end is never tried. > > `getInputBuffers()` then places both index-1 buffers into the same slot, leaving only five distinct inputs. The Java RS decoder throws: > > ``` > No enough valid inputs are provided, not recoverable > ``` > > Our targeted test uses the actual trunk `StripedReader` selection logic and Java RS decoder, with mocked readers to inject the connection failure. The control case with all primary sources available decodes successfully. The PR’s existing MD5 and Composite CRC regression tests also pass. We have not reproduced this with real DataNode failures in a cluster. > > The underlying reader behavior predates this PR. However, retaining duplicate replicas as fallbacks appears to require counting distinct internal-block indices, both during initialization and when replacing failed or slow reads. > > Does this match your understanding? Have you encountered this case, or is there an existing issue or follow-up patch? Please let us know if there is an invariant we have missed that would prevent it in practice. Thanks for the detailed reproduction. I agree with your analysis, `StripedReader` counts successful readers rather than distinct internal block indices, so it can stop before collecting enough independent inputs, both during initialization and when replacing failed or slow reads. The original goal of this PR is to prevent the checksum reconstruction target from also being selected as a source. The source reordering in the current patch only ensures that the initial candidates have distinct indices; it does not guarantee this after failures or timeouts. It can also change which fallback reads are attempted, so I think it goes beyond the scope of this fix. I have prepared a local revision that removes the reordering and the additional source count check. It only excludes entries matching the target index, preserving the order of all remaining candidates, including duplicate replicas. Your reproduction still applies after this change. I think the duplicate-source counting issue would be best addressed in a separate reader fix, with regression tests for both initialization and replacement reads. HDFS-14946 addressed the initial source ordering for NameNode scheduled reconstruction, but it does not ensure that the readers selected after connection failures or read timeouts correspond to distinct internal block indices. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
