eclipse-jgit/jgit

Fetch with protocol V2 fail if wanted ref has been updated during fetch

Closed this issue · 6 comments

thomasdraebing commented

Version

6.9.0

Operating System

Linux/Unix, MacOS

Bug description

When a large repository with a lot of refs and activity is being, it can happen that the following error is being thrown:

Caused by: org.eclipse.jgit.transport.WantNotValidException: want 9784ba017f833fc41c1ad4344a75105ee69d639e not valid
	at org.eclipse.jgit.transport.UploadPack$AdvertisedRequestValidator.checkWants(UploadPack.java:2002)
	at org.eclipse.jgit.transport.UploadPack.parseWants(UploadPack.java:1953)
	at org.eclipse.jgit.transport.UploadPack.processHaveLines(UploadPack.java:1827)
	at org.eclipse.jgit.transport.UploadPack.fetchV2(UploadPack.java:1275)
	at org.eclipse.jgit.transport.UploadPack.serveOneCommandV2(UploadPack.java:1389)
	at org.eclipse.jgit.transport.UploadPack.serviceV2(UploadPack.java:1443)
	at org.eclipse.jgit.transport.UploadPack.uploadWithExceptionPropagation(UploadPack.java:886)
	at org.eclipse.jgit.transport.UploadPack.upload(UploadPack.java:796)
	... 12 more

Initial research showed that this happens, if a wanted ref is being updated during the fetch. Looking at the code it seems that this change has to happen while the fetch spends time in UploadPack.fetchV2(), between the computation of wantedIds and advertised IDs, which happens later. In that case both Sets differ from each other and validation fails.

This was observed in Gerrit for fetches in large repositories with wide refSpecs or for mirror clones.

Actual behavior

Caused by: org.eclipse.jgit.transport.WantNotValidException: want 9784ba017f833fc41c1ad4344a75105ee69d639e not valid
	at org.eclipse.jgit.transport.UploadPack$AdvertisedRequestValidator.checkWants(UploadPack.java:2002)
	at org.eclipse.jgit.transport.UploadPack.parseWants(UploadPack.java:1953)
	at org.eclipse.jgit.transport.UploadPack.processHaveLines(UploadPack.java:1827)
	at org.eclipse.jgit.transport.UploadPack.fetchV2(UploadPack.java:1275)
	at org.eclipse.jgit.transport.UploadPack.serveOneCommandV2(UploadPack.java:1389)
	at org.eclipse.jgit.transport.UploadPack.serviceV2(UploadPack.java:1443)
	at org.eclipse.jgit.transport.UploadPack.uploadWithExceptionPropagation(UploadPack.java:886)
	at org.eclipse.jgit.transport.UploadPack.upload(UploadPack.java:796)
	... 12 more

Expected behavior

The fetch or mirror clone works and all objects are being transferred.

Relevant log output

No response

Other information

No response

msohn commented

I could now reproduce the problem using the following scenario:

  1. create Gerrit site using Gerrit master, using JGit version 692ccfc (stable-6.10)
  2. start test Gerrit server in debug mode and attach debugger
  3. set breakpoints on
    1. first statement in UploadPack.AdvertisedRequestValidator.checkWants()
    1. UploadPack.fetchV2 on line 1202 (next statement after wanted refs were computed by Map<String, ObjectId> wantedRefs = wantedRefs(req);)
  1. create test project in Gerrit
  2. create multiple changes for this project and push them for review
  3. run git clone --mirror "ssh://admin@localhost:29418/test" in one git client session, needs to use ssh protocol to ensure UploadPack.isBiDirectionalPipe() is true
  4. this session will stop on breakpoint 3.ii
  5. using another git client session amend an existing change in the test repo and push it to create a new patchset
  6. let the first client session continue by resuming the thread stopped on breakpoint 3.ii
  7. thread will stop on breakpoint 3.i
  8. step until line 2022, AdvertisedRequestValidator.checkWants() throws WantNotValidException
  9. no exception is thrown if no other change was amended after the clone computed wants in step 3.i
  10. this problem occurs also if gerrit option core.usePerRequestRefCache = true
msohn commented

Pushed fix for stable-6.10 branch for review: https://eclipse.gerrithub.io/c/eclipse-jgit/jgit/+/1201680

lucamilanesio commented

Good catch @msohn would this also make sense in older branches?

msohn commented

On which older branches do you need it ?

msohn commented