Thread (11 messages) flat view 11 messages, 2 authors, 2016-06-15

Re: [PATCH] git-p4: Use -m when running p4 changes

From: Luke Diamand <hidden>
Date: 2016-06-15 23:04:25

Possibly related (same subject, not in this thread)

On 11 April 2015 at 16:17, Lex Spoon [off-list ref] wrote:

Signed-off-by: Lex Spoon <redacted>
---
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 :-)

quoted hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help