Skip to content

Route FilterDirectory#copyFrom through createOutput (10x backport of #16530) - #16539

Open
tballison wants to merge 2 commits into
apache:branch_10xfrom
tballison:copyfrom-bookkeeping-10x
Open

Route FilterDirectory#copyFrom through createOutput (10x backport of #16530)#16539
tballison wants to merge 2 commits into
apache:branch_10xfrom
tballison:copyfrom-bookkeeping-10x

Conversation

@tballison

@tballison tballison commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Backport to branch_10x of the non-breaking parts of #16530 (which follows up #16173).

Replaces #16538.

#16173 (backported to 10x in 148ee2f) changed FilterDirectory#copyFrom to delegate to the wrapped directory, which hid FilterDirectory subclasses that override createOutput for per-file bookkeeping (TrackingTmpOutputDirectoryWrapper, MockDirectoryWrapper, NRTCachingDirectory, DirectIODirectory, ByteWritesTrackingDirectoryWrapper).

Differences from #16530, since we can't break the API on 10x:

  • Directory#copyFrom keeps its concrete default implementation (no abstract method, no copyThroughCreateOutput helper). FilterDirectory#copyFrom routes through super.copyFrom — i.e. through createOutput — instead of delegating, with javadoc noting that subclasses wanting an optimized copy (e.g. HardlinkCopyDirectoryWrapper) may override to delegate.
  • The required implementations added on main (CompoundDirectory, FileSwitchDirectory, BaseMergePolicyTestCase) are unnecessary here and are omitted.
  • TrackingTmpOutputDirectoryWrapper#copyFrom failure cleanup uses IOUtils.deleteFilesIgnoringExceptions since deleteFilesSuppressingExceptions (Catch and re-throw Throwable rather than using a success boolean #14633) is main-only.

Same as #16530:

  • Fixes TrackingTmpOutputDirectoryWrapper#copyFrom failure cleanup to delete the temp file rather than the logical dest name.
  • Ports the tests: TestFilterDirectory#testCopyFromRoutesThroughCreateOutput (replaces testCopyFromDelegates),
    TestTrackingTmpOutputDirectoryWrapper, TestByteWritesTrackingDirectoryWrapper#testCopyFromTracksBytes,
    TestDirectIODirectoryCopyFrom.

The CHANGES entry replaces the #16173 entry under 10.6.0 Bug Fixes since this reverts that behavior before it ships in a
release.

Note: Normal process would be to push this backport directly to branch_10x, I think. Since it reverts the behavior introduced by #16173, I'm opening a PR instead -- @msfroh, @iprithv, I'd appreciate your eyes on it before it goes in.

Thank you!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant