mjcrouch / vscode-perforce

Perforce commands for Visual Studio Code
MIT License
63 stars 51 forks source link

Add p4 program/version identifiers to your code #238

Open Psychobobb opened 2 years ago

Psychobobb commented 2 years ago

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

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.

mjcrouch commented 2 years ago

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!

Psychobobb commented 2 years ago

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 :

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.