On 11 April 2015 at 16:17, Lex Spoon <lex@xxxxxxxxxxxx> wrote: > > > Signed-off-by: Lex Spoon <lex@xxxxxxxxxxxx> > --- > 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 > > 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. > > 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 :-) > > 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 > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html