On Thu, Feb 25, 2010 at 11:59 AM, Dan McGee <[email protected]> wrote: > On Thu, Feb 25, 2010 at 1:46 AM, Thomas Bächler <[email protected]> wrote: >> Am 25.02.2010 02:36, schrieb Eric Bélanger: >>> This should fix the problem of the ftpdir-cleanup script removing the >>> new package instead of the old one (FS#17058). The script makes >>> $LOCK_TRIAL attempts, each separated by $LOCK_DELAY seconds, in >>> getting the repo lock. If the lock is unsuccessful, the cleanup is >>> skipped for that particular repo. >>> >>> Signed-off-by: Eric Bélanger <[email protected]> >>> >>> BTW, I didn't really tested this except the while loop logic. >>> --- >>> misc-scripts/ftpdir-cleanup | 28 +++++++++++++++++++++++++--- >>> 1 files changed, 25 insertions(+), 3 deletions(-) >>> >>> diff --git a/misc-scripts/ftpdir-cleanup b/misc-scripts/ftpdir-cleanup >>> index a185090..77f79e6 100755 >>> --- a/misc-scripts/ftpdir-cleanup >>> +++ b/misc-scripts/ftpdir-cleanup >>> @@ -10,15 +10,35 @@ dest=$2 >>> >>> ############################################################ >>> >>> -. "$(dirname $0)/../db-functions" >>> +. "$(dirname $0)/../db-functions" >>> . "$(dirname $0)/../config" >>> >>> ${CLEANUP_DRYRUN} && echo 'dry run mode is active' >>> >>> ftppath_base="$FTP_BASE/$reponame/$FTP_OS_SUFFIX" >>> >>> +LOCK_DELAY=10 >>> +LOCK_TRIAL=6 >>> + >>> for arch in ${arch...@]}; do >>> >>> + IS_LOCKED=0 >>> + count=0 >>> + while [ $count -le $LOCK_TRIAL ]; do >>> + if repo_lock $reponame $arch ; then >>> + IS_LOCKED=1 >>> + let count=$LOCK_TRIAL+1 >>> + continue >>> + fi >>> + sleep $LOCK_DELAY >>> + let count=$count+1 >>> + done >>> + >>> + if [ $IS_LOCKED -eq 0 ]; then >>> + echo "Failed to lock $reponame $arch repo" >>> + exit 1 >>> + fi >>> + >>> TMPDIR=$(mktemp -d /tmp/cleanup-XXXXXX) || exit 1 >>> ftppath="$ftppath_base/$arch" >>> MISSINGFILES="" >> >> This should be made into a generic function instead of just being in >> cleanup. Also, this seems like a duplication of kernel/glibc >> functionality - the flock(1) command should probably be used in >> repo_lock instead of the manual locking process we use. > > +1 on flock usage across the board. > > -Dan >
Yeah, I'll look into using flock in our repo_lock function instead of doing this while loop. BTW, should the timeout time be an argument of repo_lock, i.e. each script could set its own value ? Or should we use a common value (60 seconds ?) defined in config?

