| 6 years agoPerforce maintenance modified job000341 for perforce-software-p4dctl:Garbage is being added to the end of the command line when spawning p4d on Freebsd 10.2.
I tracked this down the StrRef usage in p4dctl.cpp, and was... able to get around the issue by replacing it with StrBuf. However I'm not sure what the exact cause of the issue is. « | |
|
|
| 9 years agoNick Poole commented on change 16546 for r15.2Hi @lbarbier,
I see your point: if the server is at the end of a chain of servers, then uncompressed journals are just consuming disk space.
I'm glad ...Hi @lbarbier,
I see your point: if the server is at the end of a chain of servers, then uncompressed journals are just consuming disk space.
I'm glad to hear that the 'p4 admin checkpoint' is working in your environment. I'm actually testing out a change at would use that logic for and P4D, falling back to calling 'p4d -jc' if the 'p4 admin' command failed.
The behaviour you're seeing with rotations starting at the master, and checkpoints being scheduled is by design. The journal number is stored in the counters tables, and replicated as part of the journal, so in order to ensure that journals and checkpoints match up between replicas, the journal rotation on the master causes journal rotations on the replicas. If you rotated the journal on just a replica, it would have a different journal number to the master. If the journal numbers somehow got out of sync, and you had some sort of failure that required you to restore the master, you would have no reference point to restore and restart the replica.
I agree that this could be made better, possibly by improving visibility of which replicas were scheduled to checkpoint, etc, but journal rotations are relatively cheap, so even if you rotated before you scheduled the checkpoint, and again after, I believe the overall time that the process will take will be about the same.
I'll update the review tomorrow with an option to compress the journal. I'm also going to move it into the main branch: I'll need to backport if I it's to be included in a patch release. I'll also include the 'p4 admin checkpoint/journal' calls that I mentioned (the only real difference to your shelf is that they are on the P4D class: I'm not convinced that we need another server type, and I'd rather not add unnecessary configuration options).
Thanks,
Nick « | |
|
|
| 9 years agolbarbier commented on change 16546 for r15.2Hi @nick_poole,
I picked your changes and merged into my version and pushed back into shelve #16575.
I think we need to preserve journal rotations. A ...Hi @nick_poole,
I picked your changes and merged into my version and pushed back into shelve #16575.
I think we need to preserve journal rotations. As a simple example : my edge server has 100GB of journal created each day while my commit only has 5GB a week. I want to rotate daily my edge journal so that weekly checkpoint doesn't take an hour and don't explode journal filesystem too.
I got my work working without any modification. As you said you need a privileged user but p4dctl already has all the tools it need to do so : by using P4TICKETS and P4USER in the environment section it totally fix the problem.
So now I have a replica able to checkpoint and rotate journal.
I did noticed that checkpoints weren't made on replicas until master would do a journal rotation.
I also did know that journal rotation on master would launch a rotation on replicas
It was less funny to discover that launching a replica journal rotation trigger in fact a master journal rotation. Which mean it's pointless to do a rotation of journal on replicas if something is already doing it on master. Also mean you can't rotate whenever you want a replica which IMO is a bad thing. A daily rotation on 2 sites with time difference could potentially make the rotation happen in the middle of the day.
So to do a checkpoint on a replica with an automatic task you have first to schedule the checkpoint on the replica and then launch a journal rotation on master, 2 steps need to be done in the right order.
With my current knowledge I think the whole checkpoint / rotation on replicas need a buff.
Regards,
-L « | |
|
|
| 9 years agolbarbier commented on change 16546 for mainHi Nick,
I wasn't aware of -Z for compressing the checkpoint only.
I didn't looked at journal rotation because I never do that (not yet in fact) so it ...Hi Nick,
I wasn't aware of -Z for compressing the checkpoint only.
I didn't looked at journal rotation because I never do that (not yet in fact) so it was out of my mind.
But considering it, if you want to rotate journal and compress it, shouldn't case 4 (ckp=false, jnl=true) still work for journal rotations ? I agree it would be weird to checkpoint with journal uncompressed while rotate would compress it.
Other than this, I totally agree that we then need to go further and add journal compression too.
It's your code, it might be faster for you to work on it no ?
For me now that I know the toolchain and how is the code generated it should be quite faster « | |
|
|
| 9 years agoNick Poole commented on change 16546 for mainHi @lbarbier
First off, thank you for contributing your solution back to the community.
This is a great start and I've got a couple of suggestions th ...Hi @lbarbier
First off, thank you for contributing your solution back to the community.
This is a great start and I've got a couple of suggestions that will make this feature even better.
The issue you've encountered is that 'p4dctl checkpoint' is calling 'p4d -z -jc', which is compressing the checkpoint and the rotated journal, breaking replication. But I believe that you'd also encounter the same problem if you ran 'p4dctl journal', which rotates the journal by calling 'p4d -z -jj'.
There are two solutions to this problem:
- Disable compression by not passing the '-z' flag (as in your shelf).
- Compress only the checkpoint, not the journal, by passing the '-Z' flag.
I think there are probably use cases for both, so rather than providing a single compression on/off we should add two options:
CompressCheckpoint = true/false
CompressJournal = false/true
In this the case that CompressCheckpoint == true and CompressJournal == true, pass '-z'.
In this the case that CompressCheckpoint == true and CompressJournal == false, pass '-Z'.
In this the case that CompressCheckpoint == false and CompressJournal == false, pass no flag.
In this the case that CompressCheckpoint == false and CompressJournal == true, pass no flag, or raise a config parse error.
What do you think?
If you like, I can add my suggested changes to the review for you to test out, or if you'd prefer to make the change, I'll be happy to review it.
I'd like to work with you, so I'm happy to work whichever way you'd prefer. « | |
|
|
| 9 years agoMatt Attaway commented on change 16546 for main@nick_poole will be in touch tomorrow; he's the develop for p4dctl. | |
|
|
| 9 years agolbarbier commented on change 16546 for mainThanks, I'm totally ok with it. I'm the one that asked for the feature in case #00206843
This change is "required" for users using p4dctl to create ch ...Thanks, I'm totally ok with it. I'm the one that asked for the feature in case #00206843
This change is "required" for users using p4dctl to create checkpoints in a distributed helix server (aka commit/edge) as by default they are compressed and break edge servers.
configparse.c is certainly not necessary for the CL as it's generated at compil time. « | |
|
|
| 9 years agoMatt Attaway commented on change 16546 for mainNice change! I'll talk with the developer about bringing this patch into the product if you're ok with that. | |
|
|
| 9 years agolbarbier requested review 16547 for main [ADD] Add checkpoint compression option in conf file.
In server def, add option :
CompressCkpt = false/true | |
|
|
| 9 years agoMatt Attaway committed change 16543 into main Update P4DCTL with code from 15.2 and reorg files | |
|
|
| 10 years agotyrellj created job000341 for perforce-software-p4dctl:Garbage is being added to the end of the command line when spawning p4d on Freebsd 10.2.
I tracked this down the StrRef usage in p4dctl.cpp, and was... able to get around the issue by replacing it with StrBuf. However I'm not sure what the exact cause of the issue is. « | |
|
|
| 10 years agoLester Cheung committed change 14160 into main Project README for P4DCTL. | |
|
|
| 10 years agoJason Gibson committed change 10658 into mainUpdate P4DCTL from internal Perforce tree.
Note that this change does not bring the internal Jam files and deletes the
existing builds, leaving that... as a future task.
Changes include:
################################################################################
Added support for a -o option which allows command line arguments to be passed
directly to the service being started.
################################################################################
Support for P4DTG in P4DCTL.
Example config:
p4dtg test {
Execute = /usr/local/share/p4dtg-2010.2.452775/p4dtg-repl
Owner = perforce
Environment
{
P4DTGROOT = /p4/dtg
P4DTGMAPPING = local
}
}
################################################################################
Move p4dctl's default configuration file (and the only one that is considered
safe to use) to '/etc/perforce/p4dctl.conf', and also move the directory that
gets included by default to '/etc/perforce/p4dctl.conf.d'.
################################################################################
Make p4dctl provide LANG and LC_ALL environment variables to child processes.
These both default to 'C'. They can be overridden in either global or
server-specific environment blocks, but given that 2014.2 relies on LANG, it's
better if that is set than missing.
################################################################################
Make p4dctl status exit with a code of 1 if any of the servers
that it attempts to check are not running. For automation
support
################################################################################
Change syntax of p4dctl config files so as to more clearly differentiate
between 'settings' that p4dctl understands and environment variables to be set
for the child processes.
The old style syntax looked like this:
p4d main
{
Owner = perforce
Execute = /usr/local/bin/p4d
P4ROOT = /home/perforce/p4-main
P4JOURNAL = journal
P4PORT = 1666
PATH = /bin:/usr/bin:/usr/local/bin
}
Where the only thing that differentiated the two was a convention on the use of
case. Anything p4dctl didn't understand would be set as an environment variable.
Now, we're separating the environment into a dedicated block, so p4dctl can
flag up early if it encounters settings it doesn't know about.
The new syntax looks like this:
p4d main
{
Owner = perforce
Execute = /usr/local/bin/p4d
Environment
{
P4ROOT = /home/perforce/p4-main
P4JOURNAL = journal
P4PORT = 1666
PATH = /bin:/usr/bin:/usr/local/bin
}
}
################################################################################
A new command in p4dctl that allows you to query the environment for any
configured server.
For example: list all p4d's with their Name, port and root:
$ p4dctl env -a -t p4d Name P4PORT P4ROOT
Name=p4d-master P4PORT=ssl:1666 P4ROOT=/var/lib/perforce/p4d-master
Find the P4ROOT of a known Perforce server and set P4ROOT in your
environment:
$ eval `p4dctl env -t p4d p4d-master P4ROOT`
################################################################################
Manpages for p4dctl and its config file.
################################################################################
Add 'list' command to p4dctl. Lists configured servers
visible to the current user.
Lists useful information about all the standard Perforce
Server types. « | |
|
|
| 10 years agoJason Gibson committed change 10657 into main Add a basic project description README. | |
|
|
| 11 years agoMatt Attaway committed change 8907 into mainBranch Tony Smith's p4dctl tool into perforce_software area
p4dctl is a tool that helps admins to manage multiple Perforce servers. This
tool is use...d by the Perforce Linux packages to help manage servers. « | |
|
|
