Re: [PATCH] git-p4: Use -m when running p4 changes
- From
Luke Diamand <luke@diamand.org>
- Date
- Apr 14, 2015, 11:25 UTC
- Message-ID
- <CAE5ih79UcJfuhzgTdTPy2K51sa6--4bvaVaKL3nsUcC2kq4Ffg@mail.gmail.com>
- In-Reply-To
- <CALM2SnY62u3OXJOMSqSfghH_NYwZhzSedm3-wcde-dQCX6eB9Q@mail.gmail.com>
On 11 April 2015 at 16:17, Lex Spoon <lex@lexspoon.org> wrote:
Show 9 quoted lines
> > > Signed-off-by: Lex Spoon <lex@lexspoon.org> > --- > This patch addresses a problem I am running into with a client. I am > attempting to mirror their Perforce repository into Git, and on certain > branches their Perforce server is responding with an error about "too many > rows scanned". This change has git-p4 use the "-m" option to return just 500 > changes at a time, thus avoiding the problem.
Thanks - that's a problem I also occasionally hit, and it definitely needs fixing.
Your fix is quite nice - I started out thinking this should be easy, but it's not!
A test case addition would be good if you can though - otherwise it's certain to break at some point in the future. Would you have time to add that?
Thanks! Luke
Show 5 quoted lines
> > I have tested this on a small test repository (2000 revisions) and it > appears to work fine. I have also run all the t98* tests; those print a > number of yellow "not ok" results but no red ones. I presume this is the > expected test behavior?
Yes.
Show 6 quoted lines
> > I considered making the block size configurable, but it seems unlikely > anyone will strongly benefit from changing it. 500 is large enough that it > should only take a modest number of iterations to scan the full changes > list, but it's small enough that any reasonable Perforce server should allow > the request.
Might be useful when making test harnesses though :-)
Show 63 quoted lines
>
> This patch is also available on GitHub:
> https://github.com/lexspoon/git/tree/p4-sync-batches
>
> git-p4.py | 40 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 549022e..ce1447b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -742,15 +742,41 @@ def originP4BranchesExist():
>
> def p4ChangesForPaths(depotPaths, changeRange):
> assert depotPaths
> - cmd = ['changes']
> - for p in depotPaths:
> - cmd += ["%s...%s" % (p, changeRange)]
> - output = p4_read_pipe_lines(cmd)
>
> + # Parse the change range into start and end
> + if changeRange is None or changeRange == '':
> + changeStart = '@1'
> + changeEnd = '#head'
> + else:
> + parts = changeRange.split(',')
> + assert len(parts) == 2
> + changeStart = parts[0]
> + changeEnd = parts[1]
> +
> + # Accumulate change numbers in a dictionary to avoid duplicates
> changes = {}
> - for line in output:
> - changeNum = int(line.split(" ")[1])
> - changes[changeNum] = True
> +
> + for p in depotPaths:
> + # Retrieve changes a block at a time, to prevent running
> + # into a MaxScanRows error from the server.
> + block_size = 500
> + start = changeStart
> + end = changeEnd
> + get_another_block = True
> + while get_another_block:
> + new_changes = []
> + cmd = ['changes']
> + cmd += ['-m', str(block_size)]
> + cmd += ["%s...%s,%s" % (p, start, end)]
> + for line in p4_read_pipe_lines(cmd):
> + changeNum = int(line.split(" ")[1])
> + new_changes.append(changeNum)
> + changes[changeNum] = True
> + if len(new_changes) == block_size:
> + get_another_block = True
> + end = '@' + str(min(new_changes))
> + else:
> + get_another_block = False
>
> changelist = changes.keys()
> changelist.sort()
> --
> 1.9.1
>