You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Glasser <gl...@davidglasser.net> on 2007/08/17 21:30:34 UTC
Re: svn commit: r26132 - trunk/tools/server-side
On 8/17/07, cmpilato@tigris.org <cm...@tigris.org> wrote:
> -def shard(path, max_files_per_shard):
> - """Move all the files in PATH into subdirectories of PATH named such that
> - subdirectory '0' contains at most MAX_FILES_PER_SHARD files, those named
> - [0, MAX_FILES_PER_SHARD). Abort if PATH is found to contain any entries
> - with non-numeric names."""
> +def shard(path, max_files_per_shard, start, end):
> + """Move the files for revisions START to END inclusive in PATH into
> + subdirectories of PATH named such that subdirectory '0' contains at most
> + MAX_FILES_PER_SHARD files, those named [0, MAX_FILES_PER_SHARD). Abort if
> + PATH is found to contain any entries with non-numeric names."""
>
> - # Move all entries into shards named N.shard.
> - for name in os.listdir(path):
> - try:
> - rev = int(name)
> - except ValueError, OverflowError:
> - print >> sys.stderr, "error: file '%s' does not represent a revision." \
> - % os.path.join(path, name)
> - sys.exit(1)
I think this breaks the guarantee in the doc string that it will abort
if a non-numeric entry is found. (I think that's reasonable? But the
doc string should be changed at least.)
> + tmp = path + '.reshard'
> + try:
> + os.mkdir(tmp)
> + except OSError, e:
> + if e.errno != EEXIST:
> + raise
>
> + # Move all entries into shards named N.shard.
> + for rev in xrange(start, end + 1):
> + name = str(rev)
> shard = rev // max_files_per_shard
> shard_name = str(shard) + '.shard'
>
> from_path = os.path.join(path, name)
> - to_path = os.path.join(path, shard_name, name)
> + to_path = os.path.join(tmp, shard_name, name)
> try:
> os.rename(from_path, to_path)
> except OSError:
> # The most likely explanation is that the shard directory doesn't
> # exist. Let's create it and retry the rename.
> - os.mkdir(os.path.join(path, shard_name))
> + os.mkdir(os.path.join(tmp, shard_name))
> os.rename(from_path, to_path)
>
> # Now rename all the shards to remove the suffix.
> - for name in os.listdir(path):
> + skipped = 0
> + for name in os.listdir(tmp):
> if not name.endswith('.shard'):
> print >> sys.stderr, "warning: ignoring unexpected subdirectory '%s'." \
> - % os.path.join(path, name)
> + % os.path.join(tmp, name)
> + skipped += 1
> continue
> - from_path = os.path.join(path, name)
> - to_path = from_path[:-6]
> + from_path = os.path.join(tmp, name)
> + to_path = os.path.join(path, os.path.basename(from_path)[:-6])
> os.rename(from_path, to_path)
> + skipped == 0 and os.rmdir(tmp)
Is this a common Pythonism? "if skipped == 0: os.rmdir(tmp)" certainly
makes me think less. Also, the log message implies that parallel
reshardings are reasonable, but it doesn't seem to me that this will
work (it definitely will be strange if the parallel reshardings would
affect the same shard, but even if they are building distinct shards,
this loop doesn't make sure that the shard it is installing is in the
range that it processed, so it might install an incomplete shard from
another process).
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26132 - trunk/tools/server-side
Posted by Eric Gillespie <ep...@pretzelnet.org>.
"C. Michael Pilato" <cm...@collab.net> writes:
> I can't make these changes myself right now. You want me to revert the
> previous commit, or will one of you be picking this up soon enough?
I won't be picking it up soon, but it's still better than before.
It's just still not parallelizable.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26132 - trunk/tools/server-side
Posted by David Glasser <gl...@davidglasser.net>.
On 8/20/07, C. Michael Pilato <cm...@collab.net> wrote:
> Eric Gillespie wrote:
> > "David Glasser" <gl...@davidglasser.net> writes:
> >
> >> Hmm, are you sure? Here's how I read it. Assume we're moving from
> >> linear to sharded size 1000; we run one copy on 0-1999 and one on
> >> 2000-4999, so that the first one is making shards 0 and 1, and the
> >> second is making shards 2 3 and 4. Let's say the first one completes
> >> while the second is still filling up db/revs.reshard/4.shard. The
> >> first process will happily move all five *.shard directories into
> >> db/revs and delete db/revs.reshard even while the second process is
> >> still filling 4.shard.
> >>
> >> I haven't tested this, but that's what the code looks like to me... if
> >> you don't see it, I'll try to reproduce it.
> >
> > No, you're right. I found my copy of fsfs-reshard that I had
> > actually used just left those .shard directories alone, and I
> > moved them all into place at the end. The one in my working
> > copy, with the shard renaming, I apparently never actually ran.
> > Oops! Maybe we just don't bother with the renaming if a range is
> > specified, and add a --finalize mode?
> >
> > Bleh, making tools for yourself is ever so much easier than
> > trying to generalize...
>
> I can't make these changes myself right now. You want me to revert the
> previous commit, or will one of you be picking this up soon enough?
Given that the script still complains if you try to run it, and we
have a 1.5-milestone issue for it, there's no urgency to revert.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26132 - trunk/tools/server-side
Posted by "C. Michael Pilato" <cm...@collab.net>.
Eric Gillespie wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
>
>> Hmm, are you sure? Here's how I read it. Assume we're moving from
>> linear to sharded size 1000; we run one copy on 0-1999 and one on
>> 2000-4999, so that the first one is making shards 0 and 1, and the
>> second is making shards 2 3 and 4. Let's say the first one completes
>> while the second is still filling up db/revs.reshard/4.shard. The
>> first process will happily move all five *.shard directories into
>> db/revs and delete db/revs.reshard even while the second process is
>> still filling 4.shard.
>>
>> I haven't tested this, but that's what the code looks like to me... if
>> you don't see it, I'll try to reproduce it.
>
> No, you're right. I found my copy of fsfs-reshard that I had
> actually used just left those .shard directories alone, and I
> moved them all into place at the end. The one in my working
> copy, with the shard renaming, I apparently never actually ran.
> Oops! Maybe we just don't bother with the renaming if a range is
> specified, and add a --finalize mode?
>
> Bleh, making tools for yourself is ever so much easier than
> trying to generalize...
I can't make these changes myself right now. You want me to revert the
previous commit, or will one of you be picking this up soon enough?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r26132 - trunk/tools/server-side
Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David Glasser" <gl...@davidglasser.net> writes:
> Hmm, are you sure? Here's how I read it. Assume we're moving from
> linear to sharded size 1000; we run one copy on 0-1999 and one on
> 2000-4999, so that the first one is making shards 0 and 1, and the
> second is making shards 2 3 and 4. Let's say the first one completes
> while the second is still filling up db/revs.reshard/4.shard. The
> first process will happily move all five *.shard directories into
> db/revs and delete db/revs.reshard even while the second process is
> still filling 4.shard.
>
> I haven't tested this, but that's what the code looks like to me... if
> you don't see it, I'll try to reproduce it.
No, you're right. I found my copy of fsfs-reshard that I had
actually used just left those .shard directories alone, and I
moved them all into place at the end. The one in my working
copy, with the shard renaming, I apparently never actually ran.
Oops! Maybe we just don't bother with the renaming if a range is
specified, and add a --finalize mode?
Bleh, making tools for yourself is ever so much easier than
trying to generalize...
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26132 - trunk/tools/server-side
Posted by David Glasser <gl...@davidglasser.net>.
On 8/17/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> "David Glasser" <gl...@davidglasser.net> writes:
> > Is this a common Pythonism? "if skipped == 0: os.rmdir(tmp)" certainly
> > makes me think less. Also, the log message implies that parallel
> > reshardings are reasonable, but it doesn't seem to me that this will
> > work (it definitely will be strange if the parallel reshardings would
> > affect the same shard, but even if they are building distinct shards,
> > this loop doesn't make sure that the shard it is installing is in the
> > range that it processed, so it might install an incomplete shard from
> > another process).
>
> It works if your parallel jobs are using the correct ranges, as
> mine do. We should add a check to ensure this.
Hmm, are you sure? Here's how I read it. Assume we're moving from
linear to sharded size 1000; we run one copy on 0-1999 and one on
2000-4999, so that the first one is making shards 0 and 1, and the
second is making shards 2 3 and 4. Let's say the first one completes
while the second is still filling up db/revs.reshard/4.shard. The
first process will happily move all five *.shard directories into
db/revs and delete db/revs.reshard even while the second process is
still filling 4.shard.
I haven't tested this, but that's what the code looks like to me... if
you don't see it, I'll try to reproduce it.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26132 - trunk/tools/server-side
Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David Glasser" <gl...@davidglasser.net> writes:
> On 8/17/07, cmpilato@tigris.org <cm...@tigris.org> wrote:
>
> > -def shard(path, max_files_per_shard):
> > - """Move all the files in PATH into subdirectories of PATH named such that
> > - subdirectory '0' contains at most MAX_FILES_PER_SHARD files, those named
> > - [0, MAX_FILES_PER_SHARD). Abort if PATH is found to contain any entries
> > - with non-numeric names."""
> > +def shard(path, max_files_per_shard, start, end):
> > + """Move the files for revisions START to END inclusive in PATH into
> > + subdirectories of PATH named such that subdirectory '0' contains at most
> > + MAX_FILES_PER_SHARD files, those named [0, MAX_FILES_PER_SHARD). Abort if
> > + PATH is found to contain any entries with non-numeric names."""
> >
> > - # Move all entries into shards named N.shard.
> > - for name in os.listdir(path):
> > - try:
> > - rev = int(name)
> > - except ValueError, OverflowError:
> > - print >> sys.stderr, "error: file '%s' does not represent a revision." \
> > - % os.path.join(path, name)
> > - sys.exit(1)
>
> I think this breaks the guarantee in the doc string that it will abort
> if a non-numeric entry is found. (I think that's reasonable? But the
> doc string should be changed at least.)
Yeah, just update the doc string.
> Is this a common Pythonism? "if skipped == 0: os.rmdir(tmp)" certainly
> makes me think less. Also, the log message implies that parallel
> reshardings are reasonable, but it doesn't seem to me that this will
> work (it definitely will be strange if the parallel reshardings would
> affect the same shard, but even if they are building distinct shards,
> this loop doesn't make sure that the shard it is installing is in the
> range that it processed, so it might install an incomplete shard from
> another process).
It works if your parallel jobs are using the correct ranges, as
mine do. We should add a check to ensure this.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org