vscode-perforce icon indicating copy to clipboard operation
vscode-perforce copied to clipboard

Add p4 program/version identifiers to your code

Open Psychobobb opened this issue 3 years ago • 2 comments

Is your feature request related to a problem? Please describe.

I'm a perforce admin and developer for our company and when looking into issues or monitoring - p4 program and p4 version identifiers are very useful and incredibly important to identifying problem sources quickly.

Program identifiers allow you to know the source programs that is issuing a p4 command from server side logs/monitoring.

With those present, we can build analysis or track down problematic sources on user machines.

If you do not set these in your p4 commands, by default all you see is a generic program/version identifiers (i.e. p4/2020.1/NTX64/2007551). All this tells you is that something on that machine is running a P4 CLI command and nothing more.

In an environment where you have hundreds of programs issuing perforce commands, just having generic identifiers makes it much more more difficult to troubleshoot and help users / analyze if anything is causing issues on the server.

Describe the solution you'd like

  • Add both a p4 program and version identifiers to ALL p4 commands coming from this extension. Below are the details, but once in-place, we will see the following program/version id in monitoring and server logs: vscode.mjcrouch.perforce/1.4.6

  • How to add a p4 program identifier

    • The program identifier name can be of your choice, but should be descriptive of the source (i.e. vscode.mjcrouch.perforce).
    • Avoid spaces if possible 😄
    • Please do NOT include dynamic information such as the p4 username, machine IP, p4 workspace, p4 path arguments as they are all natively in logs/monitoring and including it will mean you have unique program IDs per user/command which defeats part of the purpose.

    We just need the program name to say it came from this extension essentially so we know what on a machine is issuing the p4 commands.

    I believe you are just using P4 CLI commands (and not the P4 .NET API in any way), so in order to set this you would simply add the zprog tag to all commands (between p4 and the command to run)

    p4 -zprog=vscode.mjcrouch.perforce fstat ....
    p4 -zprog=vscode.mjcrouch.perforce change ....
    p4 -zprog=vscode.mjcrouch.perforce submit ....
    

    ☝️ Obviously, the program name should be in a variable so it's managed in one location, but above demonstrates what the p4 command being issue should look like once constructed

  • Add a version identifier also

    As programs evolve, behavior can change. So it's also important to add a version identifier so we know what version of the extension is issuing the p4 commands. This tells us if a user is running an out of date extension and whether they should upgrade to pick up bug fixes for example. Or if a new version has a bug and they need to downgrade.

    • This should be updated with each new release

    I believe you are just using P4 CLI commands (and not the P4 .NET API in any way), so in order to set this you would simply add the zversion tag to all commands alongside the zprog tag (again, between p4 and the command to run)

    p4 -zprog=vscode.mjcrouch.perforce -zversion=1.4.6 fstat ....
    p4 -zprog=vscode.mjcrouch.perforce -zversion=1.4.6 change ....
    p4 -zprog=vscode.mjcrouch.perforce -zversion=1.4.6 submit ....
    

    ☝️ Obviously, the program name and version should be in variables so it's managed in one location, but above demonstrates what the p4 command being issue should look like once constructed

Describe alternatives you've considered

There are none 😄

Additional context

We had such an issue, where a combination of this extension and the user's change list data was flattening the corporate server. As these identifiers were not set, it was much more difficult to find what on the users machine was issuing all these commands (as the user didn't know and wasn't knowingly doing anything).

Whilst this was a bad set of circumstances, it can always happen again so we would like to make sure these commands are tagged appropriately should any of our user base use this extension.

Psychobobb avatar Aug 03 '22 11:08 Psychobobb

thanks for the details! - I wasn't aware of these options and I can't find anything in the p4 command line docs except for a brief mention in logappend. Do you know where it's documented? It definitely makes sense to include them but would like to understand about e.g. supported versions and anything else to be aware of.

a combination of this extension and the user's change list data was flattening the corporate server

I'd be interested to know in generic terms what the problem was so that we can prevent it from happening in the first place, if possible!

mjcrouch avatar Aug 04 '22 14:08 mjcrouch

No problem.

Perforce's documentation is (as I'm sure you know), sometimes very difficult to navigate. There's also a lot of things they don't always add to the docs or are so difficult to find you sometimes won't find them on an easy search) and these are an example of that.

AFAIK, these aren't in their typical documentation. However :

  • Here's a KB on implementing these
  • Here's a KB on how they change the ID in the server monitor/log data

These tags whilst not really documented, AFAIK have always existed since at least P4D server version 2016.2 (I think maybe as far back as 2012.1) and are fully supported on the most recent version of the server too. I highly doubt Perforce will ever drop support for these tags, as that would impact far too many of their customers, their tools and even Perforce's own tools.


I'd be interested to know in generic terms what the problem was so that we can prevent it from happening in the first place, if possible!

Essentially a user had over 200,000+ shelved files in a changelist as part of something they were working on and they also had this extension installed in VS Code, which was minimised in the background.

The issue that arose was the extension was running thousands of p4 fstat -e <changelist_number> -Or -Rs <paths> commands (and there would be 31x paths per fstat). Given it was targeting this changelist with so many shelved files, each command would lock the server's database tables for 7-8 seconds to scan all the files for the 31x paths and then process the returned fstat data.

Whilst the database tables are locked, other user commands that need those specific tables cannot access them. So the server essentially queues the user's command, whilst it waits for the table locks to release at which point it can then use it for the next command in the queue.

As the extension was running thousands of these p4 fstat commands constantly - cumulatively it was locking the server tables for extended periods of time blocking other user commands, which essentially brought other user's access to a hold (given the table being locked were ones used heavily in most p4 commands) and the command queue grew to thousands of backed up commands rapidly.

Once we tracked down the machine/user the commands were coming from, the next challenge was to find out what on the machine was issuing them (hence the request to add zprog and zversion to all p4 commands from this extension as it would have made that a lot easier).

We believe the extension probably saw a change of some kind in the directory it was monitoring - maybe from p4v or just plain windows explorer (whilst VS code was minimised) and that was what provoked it to want to re-run fstat against all the files it could see in the directory and specifically query for shelved files against this giant the changelist they had.

Psychobobb avatar Aug 05 '22 12:08 Psychobobb