Helix SwarmHelix Swarm
Loading...

Domenic

Domenic (Domenic)
  • 0
    Followers
  • 0
    Following
  • 3
    Projects
Full Name
Domenic
Email address
domenick@bungie.com
Followers
Projects
  • Helix Installer
    • Following
  • SDP
    • Following
  • server_release_notes
    • Following
  • Activity
  • Shelves
  • Settings
  • Notifications

  • All Activity
  • Followed Activity
  • Reviews
  • Commits
  • Comments
  • Jobs
2 years agoDomenic created SDP-903 for
Broken / changed behavior in p4verify log name rotation.

In https://swarm.workshop.perforce.com/files/guest/perforce_software/sdp/dev/Server/Unix/p4/...common/bin/p4verify.sh?v=43 at line 589 the OldLogTimestamp is set to "$(ls -l --time-style +'%Y%m%d-%H%M%S' "$Log" | awk '{print $6}')". However, this does not work on (at least) Red Hat and Rocky 8.7 as the "print $6" output is the file size. Instead, $7 is needed.

In the same file at line 590 the OldLog is set to "$LOGS/p4verify.${OldLogTimestamp}.log" but prior to this change the old log file was "${LogToRotate}.${Datestamp}". As an example (after locally addressing the issue above), the new format naming results in p4verify.2023-05-09_09-40-05.log whereas the old name was p4verify.log.2023-05-09_09-28-17 so the files no longer sort the same. I *think* it was an intentional change to retain the .log extension on files but don't see it called out on https://swarm.workshop.perforce.com/projects/perforce-software-sdp/jobs/SDP-829 unless it fell under "general log file handling"?

Additionally, the previous time format was '%Y-%m-%d_%H-%M-%S' but is now '%Y%m%d-%H%M%S' so there aren't as many spacers for readability. «
Add a comment
2 years agoDomenic created SDP-853 for
After running the p4verify.sh script to validate shelved revisions there were multiple reports of "A revision range cannot be used here" in the email... / log.
However, it isn't clear *which* CLs were being tested so I'm not sure how to investigate them.

The command ran was "/p4/common/bin/p4verify.sh INSTANCENAME -nr" and the log output was:
=== Started verify of shelved changelists at Tue Nov 22 21:21:30 PST 2022.
Verifying 255528 shelved changelists.
error: A revision range cannot be used here.
exit: 1
error: A revision range cannot be used here.
exit: 1
error: A revision range cannot be used here.
exit: 1
error: A revision range cannot be used here.
exit: 1
error: A revision range cannot be used here.
exit: 1
error: A revision range cannot be used here.
exit: 1

Of 255528 shelved changes verified, 6 had errors. «
1 comment
3 years agoDomenic created SDP-810 for
Are there plans to incorporate 'p4 dbverify' into an SDP script?

I looked at the p4verify.sh script included with the SDP https://swarm.workshop.perf...orce.com/projects/perforce-software-sdp/files/dev/Server/Unix/p4/common/bin/p4verify.sh as a way to validate our metadata with dbverify https://www.perforce.com/manuals/cmdref/Content/CmdRef/p4_dbverify.html but it isn't currently included.

Before I start investigating adding it to the existing script, is it already planned to be added or setup as a separate script? If not, is there a preference for including it in p4verify.sh vs. a new script to make it easier to share back with the community? «
1 comment
3 years agoDomenic created SDP-809 for
In a failover scenario with downstream replicas (e.g.
https://swarm.workshop.perforce.com/projects/perforce-software-sdp/view/main/doc/SDP_Failover_Gu...ide.html#_resetting_downstream_replicas in addition to the 'state' file should the 'statejcopy' and 'journal.xyz' files also be renamed if they exist so that all metadata is retransferred? «
1 comment
3 years agoDomenic requested review 28902 for perforce-software-sdp:dev
Add back the info from $(date) to p4d_base so the p4d_init.log file captures the date and time that a command was ran.

It looks like this was (acci...dentally?) lost in CL 27064 when converting from "log" to "echo -e". «
Add a comment
3 years agoDomenic created SDP-775 for
P4WEB_PORT and P4FTP_PORT are not optional variables and will cause mkdirs.sh to report an "unbound variable" when trying to setup the instance vars f...ile.

In mkdirs.sh around line 576 there is a check to "Silently ignore if optional settings aren't set", which includes the P4WEB_PORT and P4FTP_PORT values. However, around line 1304 those values are used by sed and if they aren't set then multiple problems arise:
1. The script will die with "./mkdirs.sh: line 1297: P4WEB_PORT: unbound variable" and report an exit code of 0.
2. The "Failed to generate p4_${SDPInstance}.vars" error message is not reported to the user.
3. The p4_instance.vars file is not created.

The fix / workaround is just to set them in the mkdirs.cfg file even if they aren't needed / used by the instance. «
Add a comment
3 years agoDomenic created SDP-730 for
Usage of P4P_TARGET_PORT is defined but implementation is missing / confusing.

In mkdirs.cfg there is a definition for P4P_TARGET_PORT as the master'...s target port. However, in mkdirs.sh#100 there is a redefinition of that variable at line 1284 to "P4P_TARGET_PORT="${SSL_PREFIX}${P4MASTERHOST}:${P4_PORT}"" so it is used for more than just the master's target port. At line 1307 it is used by sed to replace REPL_P4P_TARGET_PORT in the instance_vars.template file, except that file doesn't have REPL_P4P_TARGET_PORT in it so ultimately P4P_TARGET_PORT is never actually used.

Looking at CL 27512 it seems the intent was to remove REPL_P4P_TARGET_PORT? If so, the mkdirs.cfg and mkdirs.sh files also have P4P_TARGET_PORT removed to avoid confusion, and perhaps a comment added to use P4PORTNUM instead of P4BROKERPORTNUM for the proxy's target port. I can do this, but first wanted to open an issue because I wasn't sure if it was the right thing to do given the check for a ServerType = p4proxy at line 1283 of mkdirs.sh. I'm also not sure if there would be impacts to other scripts or functionality if everything related to P4P_TARGET_PORT goes away although from a quick grep it looks like nothing uses it. «
Add a comment
3 years agoDomenic requested review 28485 for perforce-software-sdp:dev
Updates to upgrade.sh.
1. Move sourcing of p4_vars values before $Log definition so that the instance's log path is used rather than always writing to... /tmp. This aligns with the help info for the -L flag.

2. Expose the path to $TmpFile_1 as part of the 'p4 upgrades' step so users easily know where to go look if the process is taking longer than they'd expect rather.

3. Correct some misspellings. «
Add a comment
4 years agoDomenic created SDP-668 for
Do you have a way to automate validation of http/https links in files when packaging up the SDP so broken links can be checked with each release?

I... noticed in https://swarm.workshop.perforce.com/projects/perforce-software-sdp/files/main/doc/gen/mkrep.sh.man.txt the "Server Spec Naming Standard" and "Journal Prefix Standard" now result in 404 "page not found" errors. The new links seem to be https://swarm.workshop.perforce.com/projects/perforce-software-sdp/view/main/doc/SDP_Guide.Unix.html#_server_spec_naming_standard and https://swarm.workshop.perforce.com/projects/perforce-software-sdp/view/main/doc/SDP_Guide.Unix.html#_the_journalprefix_standard but are Unix specific. The Windows guide at https://swarm.workshop.perforce.com/projects/perforce-software-sdp/view/main/doc/SDP_Guide.Windows.html doesn't reference them.

There are 1800+ results in //guest/perforce_software/sdp/dev/doc/ for 'http' so it would be handy to have an automated checker to make sure they're all still valid each time the SDP is packaged up. «
1 comment
4 years agoDomenic created SDP-667 for
Given the impact of changes made when upgrading and the inability to rollback, would it be worth having the upgrade.sh script default to preview mode... similar to 'p4 obliterate' such that the user has to consciously add a flag to execute the work? «
2 comments
4 years agoDomenic created SDP-666 for
There doesn't appear to be a way to do an upgrade via upgrade.sh WITHOUT having a local offline_db setup.

It is possible to skip phase 1 (with -M)... but there are steps in phases 4 and 5 that reference the offline_db paths and will bail with "Journal rotation failed during offline_db upgrade phase". This means the upgrade won't complete / be considered a success and move on to later steps (i.e. starting the broker) even though p4d is up.

It would be great to have a way to skip all the offline_db checks/work, similar to how we can set DoOfflineDBTest=0 in verify_sdp.sh. «
1 comment
4 years agoDomenic created SDP-665 for
When we run upgrade.sh the script will output errors due to not having certain configurables set:
    Error: No value defined for configura...ble [any:journalPrefix].
    Error: No value defined for configurable [any:server.depot.root].

Should these really be checked at the 'any' level? Maybe we're unique, but we set these on a per-instance basis because the path names contain the instance names. E.g. /p4/sdptest/depots or /p4/sdptest-backup/depots.

Looking at //guest/perforce_software/sdp/dev/Server/setup/configure_new_server.sh it does appear to set these at the 'any' level, but that is potentially problematic down the road. E.g. if those paths aren't accessible in a multi-OS environment (having server.depot.root=z:\depot_root then bringing up an instance on Linux). «
1 comment
4 years agoDomenic requested review 27884 for perforce-software-sdp:dev
Add closing brace for TargetServerID variable in the error message.
1 comment
4 years agoDomenic commented on review 27776 for perforce-software-sdp:dev

Gah - you told me about updating the Version recently and I already forgot :( Sorry! Thanks for reviewing this!

2 comments
4 years agoDomenic requested review 27776 for perforce-software-sdp:dev
Add a check for REPLICA_ID matching MASTER_ID to ensure that replica/edge servers don't use the same setting as the master server.

Anchor the check...s for SERVER_TYPE so that leading/trailing characters aren't included. This also meant adding an explicit entry for p4d_edge_replica as it was previously implicit from p4d_edge. «
2 comments
4 years agoDomenic commented on review 27712 for perforce-software-sdp:dev, tools

Thanks for the note on auto-generated documentation - I should have asked about that before diving in (or paid closer attention to the Makefile :))..

8 comments
4 years agoDomenic commented on review 27712 (upgrade.sh) for perforce-software-sdp:dev, tools

Ah, thanks Tom! I had totally spaced on even thinking about that :(

8 comments
4 years agoDomenic updated files in review 27712 for perforce-software-sdp:dev, tools
Pass to address spelling issues.
Most of these are just in comments or user-facing messages, but there were a couple that could impact script behavior.... E.g. Server/test/test_SDP.py and Unsupported/Maintenance/sdputils.py looking at files or paths that don't exist.

Other notes..
1. I did not do anything with PDF files as I assume those are automatically generated.
2. This pass did address SDP-637, although I did it in the .adoc and .html files instead of the PDF.
3. I eventually realized Robert may have written some of these but I'd already saved the files changing things like "customisation" -> "customization" or "behaviour" -> "behavior". Sorry :( «
8 comments
4 years agoDomenic requested review 27712 for perforce-software-sdp:dev, tools
Pass to address spelling issues.
Most of these are just in comments or user-facing messages, but there were a couple that could impact script behavior.... E.g. Server/test/test_SDP.py and Unsupported/Maintenance/sdputils.py looking at files or paths that don't exist.

Other notes..
1. I did not do anything with PDF files as I assume those are automatically generated.
2. This pass did address SDP-637, although I did it in the .adoc and .html files instead of the PDF.
3. I eventually realized Robert may have written some of these but I'd already saved the files changing things like "customisation" -> "customization" or "behaviour" -> "behavior". Sorry :( «
8 comments
4 years agoDomenic commented on SDP-442 for

Just wanted to point out that disabling THP has changed in RHEL8. I opened a support case about it (00780768) and John updated the KB article at https

...

Just wanted to point out that disabling THP has changed in RHEL8. I opened a support case about it (00780768) and John updated the KB article at https://community.perforce.com/s/article/3005 to note that grub is the preferred way to handle this.

 «
4 comments
10 years agoDomenic commented on review 13886 (p4p_instance_init.template, line 34) for perforce-software-sdp:main

Maybe this review isn't the right forum for this question, but what the heck.. :)

I'm curious why the proxy specific values are included in the templa

...

Maybe this review isn't the right forum for this question, but what the heck.. :)

I'm curious why the proxy specific values are included in the template that all services pick up rather than the specific p4p template so they aren't being exported if no proxy is running for that instance? I'm still getting familiar with the SDP so I apologize if the answer is obvious :)

 «
4 comments
10 years agoDomenic commented on review 13886 (p4p_instance_init.template, line 34) for perforce-software-sdp:main

Ah, I think I see what happened..

I downloaded the latest SDP release (sdp.Unix.2015.1.12399.tgz; CL 12401) and used that to setup some proxies. It lo

...

Ah, I think I see what happened..

I downloaded the latest SDP release (sdp.Unix.2015.1.12399.tgz; CL 12401) and used that to setup some proxies. It looks like the missing proxy variables were added back in CL 12443 but a new SDP hasn't been posted to https://swarm.workshop.perforce.com/projects/perforce-software-sdp/files/downloads that includes them. Once a new one is posted then yeah, this change is redundant / not needed.

 «
4 comments
10 years agoDomenic requested review 13886 for perforce-software-sdp:main
Changes for proxy setup / support.
- PROXY_PORT and PROXY_TARGET are supported during SDP setup and set via per-instance templates.
- Set a default v...alue in p4p_instance_init when referencing p4p_base so 'service p4p_X_init' won't report $1 is an unbound variable. «
4 comments
10 years agoDomenic requested review 10850 for perforce-software-sdp:dev
Base proxy script updates.
- Remove p4broker references.
- Fix "stop" command for the service to get p4p_pids instead of p4broker_pids.
Add a comment
Change User Description Created
28899 Domenic Add back the info from $(date) to p4d_base so the p4d_init.log file captures the date and... time that a command was ran.

It looks like this was (accidentally?) lost in CL 27064 when converting from "log" to "echo -e".

#review-28902 «		 3 years ago View Review
28484 Domenic Updates to upgrade.sh.
1. Move sourcing of p4_vars values before $Log definition so that t...he instance's log path is used rather than always writing to /tmp. This aligns with the help info for the -L flag.

2. Expose the path to $TmpFile_1 as part of the 'p4 upgrades' step so users easily know where to go look if the process is taking longer than they'd expect.

3. Correct some misspellings.

#review-28485 «		 3 years ago View Review
27883 Domenic Add closing brace for TargetServerID variable in the error message.

#review-27884 		4 years ago View Review
27775 Domenic Add a check for REPLICA_ID matching MASTER_ID to ensure that replica/edge servers don't us...e the same setting as the master server.

Anchor the checks for SERVER_TYPE so that leading/trailing characters aren't included. This also meant adding an explicit entry for p4d_edge_replica as it was previously implicit from p4d_edge.

#review-27776 «		 4 years ago View Review
27711 Domenic Pass to address spelling issues.
Most of these are just in comments or user-facing messag...es, but there were a couple that could impact script behavior. E.g. Server/test/test_SDP.py and Unsupported/Maintenance/sdputils.py looking at files or paths that don't exist.

Other notes..
1. I did not do anything with PDF files as I assume those are automatically generated.
2. This pass did address SDP-637, although I did it in the .adoc and .html files instead of the PDF.
3. I eventually realized Robert may have written some of these but I'd already saved the files changing things like "customisation" -> "customization" or "behaviour" -> "behavior". Sorry :(

#review-27712 «		 4 years ago View Review
10849 Domenic Base proxy script updates.
- Remove p4broker references.
- Fix "stop" command for the ser...vice to get p4p_pids instead of p4broker_pids.

[review-10850] «		 10 years ago View Review

Profile Settings

Reset to default

Reviews

Sets default values for how diffs are displayed on all reviews.

Show comments in files
View diffs side-by-side
Show space and newline characters
Ignore whitespace when calculating differences

Time Display

Sets how you would like to display time across Swarm.

Display the time in

Adjust when notifications are sent to you about reviews that you're associated with (as an author, reviewer, project member or moderator).

Email me when:

Reset to default
Check all
I change the state of a review
I am the author, and
a review is requested
files in the review are updated
tests on the review have finished
a vote is cast on a review
the state of the review changes
a review or change is committed
a comment is made on the review or change
a comment on the review or change is updated
Someone likes one of my comments
I am a member, and
a review is requested
a review or change is committed
I am a reviewer, and
files in the review are updated
tests on the review have finished
a vote is cast on a review
the state of the review changes
someone joins or leaves the review
a review or change is committed
a comment is made on the review
a comment on the review is updated
I am a moderator, and
files in the review are updated
tests on the review have finished
a review or change is committed