Ashwani Raina has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24681 )

Change subject: [dist-test] Fix chrpath ETXTBSY race in dist slaves
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/24681/3//COMMIT_MSG
Commit Message:

PS3:
> Thank you for the root cause analysis!
>From testing outcome, I didn't notice any other issue in dist-test vis-à-vis 
>KUDU-3788 changes. But, that is based on the targeted testing (long running 
>tests) I did for ENOSPC issue.

However, I did found some issues that are theoretically possible. I have tried 
to address those in latest PS.

To answer your question about switching - I see no reason to switch to legacy 
mode and lose out on benefits that KUDU-3788 changes provide. Besides, if there 
are any other issues found during the course, we could fix those as we go 
along, unless we discover something that is either hard to fix (effort-wise) or 
is not fixable.


http://gerrit.cloudera.org:8080/#/c/24681/3//COMMIT_MSG@10
PS3, Line 10: open
> nit: open() call
Done


http://gerrit.cloudera.org:8080/#/c/24681/3//COMMIT_MSG@16
PS3, Line 16: lvm-symbolizer
> BTW, maybe we should avoid calling chrpath for llvm-symbolizer (and maybe f
This is addressed in latest PS. Instead of calling chrpath blindly on every 
binary, fix_rpath() skips the rewrite if RPATH is already relativized.


http://gerrit.cloudera.org:8080/#/c/24681/3/build-support/run_dist_test.py
File build-support/run_dist_test.py:

http://gerrit.cloudera.org:8080/#/c/24681/3/build-support/run_dist_test.py@110
PS3, Line 110: Work around this by writing to a temporary copy and
             :   # atomically replacing the original via os.rename(), which 
only updates
             :   # the directory entry and leaves any in-flight execution of 
the original
             :   # inode undisturbed. A process-unique suffix avoids a 
collision when two
             :   # tasks concurrently fall back for the same binary.
> Would it be cleaner to do this unconditionally before trying to run "chrpat
Doing this unconditionally would not be wise as fix_rpath() is called for all 
the ELF files in the directory tree. Copying would incur double transient disk 
cost and additional read & write cycle per file. All this for a cheaper rewrite 
of rpaths using 'chrpath'.

Also, hitting such collision (ETXTBSY error) is rare already that will become 
even rarer with latest PS fix which completely skips rwrite for the binaries 
that already contain relativized rpaths.


http://gerrit.cloudera.org:8080/#/c/24681/3/build-support/run_dist_test.py@117
PS3, Line 117: CalledProcessError
> nit: Is there a way to confirm the error is ETXTBSY before going down this
Good point!
That makes sense as selective handling would avoid masking non-ETXTBSY errors.


http://gerrit.cloudera.org:8080/#/c/24681/3/build-support/run_dist_test.py@118
PS3, Line 118:     tmp_path = "%s.chrpath_tmp.%d" % (path, os.getpid())
> Maybe add some logging to help any future debugging
Done



--
To view, visit http://gerrit.cloudera.org:8080/24681
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80cdc5b903f5ed0865ec5a92f905a46719f7e316
Gerrit-Change-Number: 24681
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Martonka <[email protected]>
Gerrit-Comment-Date: Mon, 17 Aug 2026 13:30:54 +0000
Gerrit-HasComments: Yes

Reply via email to