[arch-dev-public] [PATCH 4/4] ftpdir-cleanup: lock repo before doing the cleanup

[Eric Bélanger]

On Thu, Feb 25, 2010 at 11:59 AM, Dan McGee <dpmcgee at gmail.com> wrote:
> On Thu, Feb 25, 2010 at 1:46 AM, Thomas Bächler <thomas at archlinux.org> 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 <snowmaniscool at gmail.com>
>>>
>>> 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 ${ARCHES[@]}; 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?