Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-65246

P4 Polling does not work correctly if changelist is specified during sync

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      If a changelist is specified in scm: checkout, p4 polling does not work.  The result is the specified changelist is used in the range:  

       

      P4: Polling with range: 53910841,53910841 

      p4 changes -m20 //clientname_/...@53910841,53910841

       

      This result, is polling will not detect any subsequent changes.  

       

      If changelist is not specified during scm: checkout, 

       

      P4: Polling with range: 53910841,now 

      p4 changes -m20 //clientname_/...@53910841,now

       

      P4 plugin should always use "now" for the higher end of the range, regardless if a changelist was specified or not.  

       

       

       

        Attachments

          Activity

          Hide
          p4karl Karl Wirth added a comment -

          Hi Al Watson

          Please can you show my how you are specifying the changelist in the sync. Is this using pipeline/multibranch pipeline.

          Thanks in advance.

          Show
          p4karl Karl Wirth added a comment - Hi Al Watson Please can you show my how you are specifying the changelist in the sync. Is this using pipeline/multibranch pipeline. Thanks in advance.
          Hide
          atwatsoniii Al Watson added a comment -

          Using declarative pipeline:

           

          checkout perforce(

          populate:

          syncOnly(
          force: false,
          have: true,
          modtime: false,
          parallel: [enable: true, minbytes: '1024', minfiles: '1', threads: '4'],
          pin: "${env.CHANGELIST}",
          quiet: true,
          revert: true)

           

           

          Show
          atwatsoniii Al Watson added a comment - Using declarative pipeline:   checkout perforce( populate: syncOnly( force: false, have: true, modtime: false, parallel: [enable: true, minbytes: '1024', minfiles: '1', threads: '4'] , pin: "${env.CHANGELIST}", quiet: true, revert: true)    
          Hide
          p4karl Karl Wirth added a comment -

          Hi Al Watson,

          Thanks. Why are you trying to pin a polled job? Are you trying to implement 'pin at polled' - JENKINS-63879?

          Usually you want polling to find the highest changeslist and then sync to that highest changelist which is the way the code expects. The main reason you dont want to is when the job takes a long time to run on a busy system and contains multiple sync steps that need to be consistent which is what the above enhancement is trying to implement.

          Does your job have one or more than one sync/checkout step? If more than one sync step do you need polling for all the code paths including the pinned one?

          Regards,

          Karl

           

          Show
          p4karl Karl Wirth added a comment - Hi Al Watson , Thanks. Why are you trying to pin a polled job? Are you trying to implement 'pin at polled' - JENKINS-63879 ? Usually you want polling to find the highest changeslist and then sync to that highest changelist which is the way the code expects. The main reason you dont want to is when the job takes a long time to run on a busy system and contains multiple sync steps that need to be consistent which is what the above enhancement is trying to implement. Does your job have one or more than one sync/checkout step? If more than one sync step do you need polling for all the code paths including the pinned one? Regards, Karl  
          Hide
          atwatsoniii Al Watson added a comment -

          I only have one sync step that I want to do polling on, and that's the example above.   However, as an example, I may initiate a manual run at a specific changelist.  In the current paradigm, doing that will disable polling until a run is made without "pin".   What is the reason behind the current methodology?  It seems this behavior is always undesirable, it effectively disables polling.  

          Show
          atwatsoniii Al Watson added a comment - I only have one sync step that I want to do polling on, and that's the example above.   However, as an example, I may initiate a manual run at a specific changelist.  In the current paradigm, doing that will disable polling until a run is made without "pin".   What is the reason behind the current methodology?  It seems this behavior is always undesirable, it effectively disables polling.  
          Hide
          atwatsoniii Al Watson added a comment -

          Any updates or questions?

          Show
          atwatsoniii Al Watson added a comment - Any updates or questions?
          Hide
          kyates Keith Yates added a comment -

          I'm also thinking this isn't much of a 'minor' issue. In my case I have one primary job that triggers a bunch of sub-jobs. The primary job polls and gets the changelist, then passes that to the sub-jobs (which handle their own email/notifications to committers) so they need to use that changelist to get who is responsible. And all the jobs are capable to have changelist passed in as a parameter as well. And they are setup to also be able to run independently.

          So I really do need to be able to pass along a changelist and have it not break polling as a result.

          Show
          kyates Keith Yates added a comment - I'm also thinking this isn't much of a 'minor' issue. In my case I have one primary job that triggers a bunch of sub-jobs. The primary job polls and gets the changelist, then passes that to the sub-jobs (which handle their own email/notifications to committers) so they need to use that changelist to get who is responsible. And all the jobs are capable to have changelist passed in as a parameter as well. And they are setup to also be able to run independently. So I really do need to be able to pass along a changelist and have it not break polling as a result.
          Hide
          kyates Keith Yates added a comment - - edited

          I'm going to move this to Major as it means you can result with changes in the 'blame' list that aren't part of a build, as you have to always poll latest even if you are syncing to a specific build.

          meaning you have to do a sync with no changelist to get polling working, but if you initiate a build to a specific changelist that will be a seperate sync so polling would keep working

          Show
          kyates Keith Yates added a comment - - edited I'm going to move this to Major as it means you can result with changes in the 'blame' list that aren't part of a build, as you have to always poll latest even if you are syncing to a specific build. meaning you have to do a sync with no changelist to get polling working, but if you initiate a build to a specific changelist that will be a seperate sync so polling would keep working
          Hide
          p4karl Karl Wirth added a comment -

          Hi Keith Yates

          The way polling works is that it looks in the last succesful build build.xml file and then should look for changes since then. For example I used the Perforce built in label 'now' during build 13. The  contents of JENKINS_HOME/jobs/JOB_NAME/builds/13/build.xml is:

            <refChanges>
                  <org.jenkinsci.plugins.p4.changes.P4LabelRef>
                    <label>now</label>
                  </org.jenkinsci.plugins.p4.changes.P4LabelRef>
           </refChanges>
          

          when I manually run the polling I see:

          Found last change now on syncID jenkins-NODE_NAME-PinPolledFreestyle-EXECUTOR_NUMBER
          

          If change 13 had built an actual change instead of a label the build.xml contains that highest change. For example:

                <refChanges>
                  <org.jenkinsci.plugins.p4.changes.P4ChangeRef>
                    <change>80</change>
                  </org.jenkinsci.plugins.p4.changes.P4ChangeRef>
                </refChanges>
          

          So next time it gets stuck can you please get me the 'build.xml' for the last succesful job and the polling log after that which does not find anything.

          Thanks in advance,

          Karl

           

           

          Show
          p4karl Karl Wirth added a comment - Hi Keith Yates The way polling works is that it looks in the last succesful build build.xml file and then should look for changes since then. For example I used the Perforce built in label 'now' during build 13. The  contents of JENKINS_HOME/jobs/JOB_NAME/builds/13/build.xml is: <refChanges> <org.jenkinsci.plugins.p4.changes.P4LabelRef> <label>now</label> </org.jenkinsci.plugins.p4.changes.P4LabelRef> </refChanges> when I manually run the polling I see: Found last change now on syncID jenkins-NODE_NAME-PinPolledFreestyle-EXECUTOR_NUMBER If change 13 had built an actual change instead of a label the build.xml contains that highest change. For example: <refChanges> <org.jenkinsci.plugins.p4.changes.P4ChangeRef> <change>80</change> </org.jenkinsci.plugins.p4.changes.P4ChangeRef> </refChanges> So next time it gets stuck can you please get me the 'build.xml' for the last succesful job and the polling log after that which does not find anything. Thanks in advance, Karl    
          Hide
          kyates Keith Yates added a comment - - edited

          Karl Wirth, I provided 288148 and that's what the build.xml shows.

              <org.jenkinsci.plugins.p4.tagging.TagAction plugin="p4@1.11.5">
                <tags/>
                <refChanges>
                  <org.jenkinsci.plugins.p4.changes.P4ChangeRef>
                    <change>288148</change>
                  </org.jenkinsci.plugins.p4.changes.P4ChangeRef>
                </refChanges>
          

          Leaving it as an empty string results in the same changes to build.xml. I use the empty string for the latest, I've never used 'now'.

          Here's the polling log even though 288159 has changes, the polling isn't polling anything past the provided changelist.

          P4: Polling with range: 288148,288148
          ...tick...
          ... p4 changes -m10 //*****/...@288148,288148
           +
          ... p4 repos -C
           +
          P4: Polling no changes found.
          P4: Polling no changes found.
          no polling baseline in *****
          
          Show
          kyates Keith Yates added a comment - - edited Karl Wirth , I provided 288148 and that's what the build.xml shows. <org.jenkinsci.plugins.p4.tagging.TagAction plugin= "p4@1.11.5" > <tags/> <refChanges> <org.jenkinsci.plugins.p4.changes.P4ChangeRef> <change>288148</change> </org.jenkinsci.plugins.p4.changes.P4ChangeRef> </refChanges> Leaving it as an empty string results in the same changes to build.xml. I use the empty string for the latest, I've never used 'now'. Here's the polling log even though 288159 has changes, the polling isn't polling anything past the provided changelist. P4: Polling with range: 288148,288148 ...tick... ... p4 changes -m10 / /*****/ ...@288148,288148 + ... p4 repos -C + P4: Polling no changes found. P4: Polling no changes found. no polling baseline in *****
          Hide
          kyates Keith Yates added a comment - - edited

          Karl Wirth,

          I think the issue is in PollTask.java, this usage of the 'pin' which is how you currently sync to a change in the p4 plugin is by using the pin. I think it shouldn't be using the pin at all and should just as the original reporter suggested, always poll latest.

          Because pin is how you sync to a specific change, once you've done it polling will forever try to check for changes with that being the reference and the thing to compare against.

          	@Override
          	public Object task(ClientHelper p4) throws Exception {
          		List<P4Ref> changes = new ArrayList<P4Ref>();
          
          		// find changes...
          		if (pin != null && !pin.isEmpty()) {
          			changes = p4.listHaveChanges(lastRefs, new P4LabelRef(pin));
          		} else {
          			changes = p4.listHaveChanges(lastRefs);
          		}
          

          It's getting that from PerforceScm.java:

          	private List<P4Ref> lookForChanges(FilePath buildWorkspace, Workspace ws, Run<?, ?> lastRun, TaskListener listener)
          			throws IOException, InterruptedException {
          
          		// Set EXPANDED client
          		String syncID = ws.getSyncID();
          
          		// Set EXPANDED pinned label/change
          		String pin = populate.getPin();
          		if (pin != null && !pin.isEmpty()) {
          			pin = ws.getExpand().format(pin, false);
          			ws.getExpand().set(ReviewProp.P4_LABEL.toString(), pin);
          		}
          
          		// Calculate last change, build if null (JENKINS-40356)
          		List<P4Ref> lastRefs = TagAction.getLastChange(lastRun, listener, syncID);
          		if (lastRefs == null || lastRefs.isEmpty()) {
          			// no previous build, return null.
          			listener.getLogger().println("P4: Polling: No changes in previous build.");
          			return null;
          		}
          
          		// Create task
          		PollTask task = new PollTask(credential, lastRun, listener, filter, lastRefs);
          		task.setWorkspace(ws);
          		task.setLimit(pin);
          
          		// Execute remote task
          		List<P4Ref> changes = buildWorkspace.act(task);
          		return changes;
          	}
          
          Show
          kyates Keith Yates added a comment - - edited Karl Wirth , I think the issue is in PollTask.java, this usage of the 'pin' which is how you currently sync to a change in the p4 plugin is by using the pin. I think it shouldn't be using the pin at all and should just as the original reporter suggested, always poll latest. Because pin is how you sync to a specific change, once you've done it polling will forever try to check for changes with that being the reference and the thing to compare against. @Override public Object task(ClientHelper p4) throws Exception { List<P4Ref> changes = new ArrayList<P4Ref>(); // find changes... if (pin != null && !pin.isEmpty()) { changes = p4.listHaveChanges(lastRefs, new P4LabelRef(pin)); } else { changes = p4.listHaveChanges(lastRefs); } It's getting that from PerforceScm.java: private List<P4Ref> lookForChanges(FilePath buildWorkspace, Workspace ws, Run<?, ?> lastRun, TaskListener listener) throws IOException, InterruptedException { // Set EXPANDED client String syncID = ws.getSyncID(); // Set EXPANDED pinned label/change String pin = populate.getPin(); if (pin != null && !pin.isEmpty()) { pin = ws.getExpand().format(pin, false ); ws.getExpand().set(ReviewProp.P4_LABEL.toString(), pin); } // Calculate last change, build if null (JENKINS-40356) List<P4Ref> lastRefs = TagAction.getLastChange(lastRun, listener, syncID); if (lastRefs == null || lastRefs.isEmpty()) { // no previous build, return null . listener.getLogger().println( "P4: Polling: No changes in previous build." ); return null ; } // Create task PollTask task = new PollTask(credential, lastRun, listener, filter, lastRefs); task.setWorkspace(ws); task.setLimit(pin); // Execute remote task List<P4Ref> changes = buildWorkspace.act(task); return changes; }
          Hide
          p4karl Karl Wirth added a comment -

          Hi Keith Yates,

          Thanks. I was having problems reproducing it, but just managed it. Assigning to the devs. They will review the defect during their next sprint review. I do not know when this will be but I will also highlight this to the product manager.

           

          Reproduction steps for Dev:

          (1) Create the following directory structure in Perforce:

           

          //depot/PinPolled2/Jenkinsfile/Jenkinsfile
          //depot/PinPolled2/src/f1
          //depot/PinPolled2/src/f2

          and use the Jenkinsfile:

           

          pipeline {
            agent { label 'master' }
            stages {
              stage("Repro") {
                steps {
                  script {
          	checkout perforce(credential: 'JenkinsMaster', populate: forceClean(have: false, parallel: [enable: false, minbytes: '1024', minfiles: '1', threads: '4'], pin: params.PINCL, quiet: false), workspace: manualSpec(charset: 'none', cleanup: false, name: 'jenkins-${NODE_NAME}-${JOB_NAME}-${EXECUTOR_NUMBER}-SRC', pinHost: false, spec: clientSpec(allwrite: false, backup: true, changeView: '', clobber: true, compress: false, line: 'LOCAL', locked: false, modtime: false, rmdir: false, serverID: '', streamName: '', type: 'WRITABLE', view: '//depot/PinPolled2/src/... //${P4_CLIENT}/...')))
                  }
                } 
              }
            }
          }
          

          (2) Create a new pipeline job with the string paramater "PINCL" with no default value and Jenkinsfile from SCM source:

          //depot/PinPolled2/Jenkinsfile/... //${P4_CLIENT}/...
          

          (3) Manually build the job with no paramater to set up polling.

          (4) Submit a new file to 'src' subtree.

          (5) Run 'Poll Now' (assumes poll now plugin installed).

          (6) Correct changelist found.

          (7) Submit two new files to 'src' subtree in seperate changelists.

          (8) Manually build the job with paramater set to first changelist from step (7). Therefore we have not built head.

          (9) Run 'Poll now' again. The changelist from step (8) is used as the lower and upper limit. For example it was CL 123 (pure coincidence ) in my testing and head revision for 'src' was CL 124:

          Perforce Software Polling Log
          
          Started on Jun 9, 2021 11:36:03 AM
          Executor number at runtime: 0
          P4: Polling on: master with:jenkins-master-PinPolled2JenkinsfileNew-0
          Found last change 121 on syncID jenkins-NODE_NAME-PinPolled2JenkinsfileNew-EXECUTOR_NUMBER
          ... p4 login -s +
          ... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0 +
          ... p4 info +
          ... p4 info +... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0 +
          P4 Task: establishing connection.
          ... server: 192.168.1.160:1666
          ... node: vm-kwirth-swarm202-xenial
          P4: Polling with range: 121,now
          ... p4 changes -m20 //jenkins-master-PinPolled2JenkinsfileNew-0/...@121,now +
          ... p4 repos -C +
          P4: Polling no changes found.
          
          Executor number at runtime: 0
          P4: Polling on: master with:jenkins-master-PinPolled2JenkinsfileNew-0-SRC
          Found last change 123 on syncID jenkins-NODE_NAME-PinPolled2JenkinsfileNew-EXECUTOR_NUMBER-SRC
          ... p4 login -s +
          ... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0-SRC +
          ... p4 info +
          ... p4 info +
          ... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0-SRC +
          P4 Task: establishing connection.
          ... server: 192.168.1.160:1666
          ... node: vm-kwirth-swarm202-xenial
          P4: Polling with range: 123,123
          ... p4 changes -m20 //jenkins-master-PinPolled2JenkinsfileNew-0-SRC/...@123,123 +
          ... p4 repos -C +P4: Polling no changes found.
          Done. Took 55 ms
          No changes
          

          Note the "p4 changes -m20 //jenkins-master-PinPolled2JenkinsfileNew-0-SRC/...@123,123".

           

          Show
          p4karl Karl Wirth added a comment - Hi Keith Yates , Thanks. I was having problems reproducing it, but just managed it. Assigning to the devs. They will review the defect during their next sprint review. I do not know when this will be but I will also highlight this to the product manager.   Reproduction steps for Dev: (1) Create the following directory structure in Perforce:   //depot/PinPolled2/Jenkinsfile/Jenkinsfile //depot/PinPolled2/src/f1 //depot/PinPolled2/src/f2 and use the Jenkinsfile:   pipeline { agent { label 'master' } stages { stage( "Repro" ) { steps { script { checkout perforce(credential: 'JenkinsMaster' , populate: forceClean(have: false , parallel: [enable: false , minbytes: '1024' , minfiles: '1' , threads: '4' ], pin: params.PINCL, quiet: false ), workspace: manualSpec(charset: 'none' , cleanup: false , name: 'jenkins-${NODE_NAME}-${JOB_NAME}-${EXECUTOR_NUMBER}-SRC' , pinHost: false , spec: clientSpec(allwrite: false , backup: true , changeView: '', clobber: true , compress: false , line: ' LOCAL ', locked: false , modtime: false , rmdir: false , serverID: ' ', streamName: ' ', type: ' WRITABLE ', view: ' //depot/PinPolled2/src/... //${P4_CLIENT}/...'))) } } } } } (2) Create a new pipeline job with the string paramater "PINCL" with no default value and Jenkinsfile from SCM source: //depot/PinPolled2/Jenkinsfile/... //${P4_CLIENT}/... (3) Manually build the job with no paramater to set up polling. (4) Submit a new file to 'src' subtree. (5) Run 'Poll Now' (assumes poll now plugin installed). (6) Correct changelist found. (7) Submit two new files to 'src' subtree in seperate changelists. (8) Manually build the job with paramater set to first changelist from step (7). Therefore we have not built head. (9) Run 'Poll now' again. The changelist from step (8) is used as the lower and upper limit. For example it was CL 123 (pure coincidence ) in my testing and head revision for 'src' was CL 124: Perforce Software Polling Log Started on Jun 9, 2021 11:36:03 AM Executor number at runtime: 0 P4: Polling on: master with:jenkins-master-PinPolled2JenkinsfileNew-0 Found last change 121 on syncID jenkins-NODE_NAME-PinPolled2JenkinsfileNew-EXECUTOR_NUMBER ... p4 login -s + ... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0 + ... p4 info + ... p4 info +... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0 + P4 Task: establishing connection. ... server: 192.168.1.160:1666 ... node: vm-kwirth-swarm202-xenial P4: Polling with range: 121,now ... p4 changes -m20 //jenkins-master-PinPolled2JenkinsfileNew-0/...@121,now + ... p4 repos -C + P4: Polling no changes found. Executor number at runtime: 0 P4: Polling on: master with:jenkins-master-PinPolled2JenkinsfileNew-0-SRC Found last change 123 on syncID jenkins-NODE_NAME-PinPolled2JenkinsfileNew-EXECUTOR_NUMBER-SRC ... p4 login -s + ... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0-SRC + ... p4 info + ... p4 info + ... p4 client -o jenkins-master-PinPolled2JenkinsfileNew-0-SRC + P4 Task: establishing connection. ... server: 192.168.1.160:1666 ... node: vm-kwirth-swarm202-xenial P4: Polling with range: 123,123 ... p4 changes -m20 //jenkins-master-PinPolled2JenkinsfileNew-0-SRC/...@123,123 + ... p4 repos -C +P4: Polling no changes found. Done. Took 55 ms No changes Note the "p4 changes -m20 //jenkins-master-PinPolled2JenkinsfileNew-0-SRC/...@123,123".  
          Hide
          atwatsoniii Al Watson added a comment -

          Any updates?

          Show
          atwatsoniii Al Watson added a comment - Any updates?
          Hide
          atwatsoniii Al Watson added a comment -

          Ping, Any Updates for this issue?

          Show
          atwatsoniii Al Watson added a comment - Ping, Any Updates for this issue?

            People

            Assignee:
            p4karl Karl Wirth
            Reporter:
            atwatsoniii Al Watson
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated: