New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix some compiler and doxygen warnings #179
Conversation
3a4ab4c
to
f170e00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow Mattias (@ellert), that must have been a long and demanding task. Much obliged that you went after these compiler warnings. This PR looks like a huge improvement in quality for the GCT.
Sorry, but the review became a long haul for me. And it looks like I can't comment on parts of the changes separate from the review when I already started a review, so my comments are not visible until after I make my review comment. So again, sorry for taking so long.
For the change in the mutex (un)locking in gridftp/server/src/globus_gridftp_server.c
after a lot of tries I managed to get a situation which looked like a deadlock. I performed test runs in a single VM with 6 virtual CPUs attached that generates more than 400 threads for a single transfer. One test run took too long compared to other test runs with practically no load on the CPUs, so I cancelled it and found myself in the following situation:
## Standard number of processes:
## 6 DTPs
## 1 PI
## 1 line for `grep` command
[johndoe@gridftp-5 ~]$ ps auxf | grep globus-gridftp-server | wc -l
8
## After cancelling the test
[johndoe@gridftp-5 ~]$ ps auxf | grep globus-gridftp-server | wc -l
14
## After stopping the gridftpd...
[johndoe@gridftp-5 ~]$ sudo gridftpd/etc/init.d/gridftpd stop
[sudo] password for johndoe:
Stopping gridftpd: [ OK ]
## ...there are still 5 DTPs and 1 PI running
[johndoe@gridftp-5 ~]$ ps auxf | grep globus-gridftp-server | wc -l
7
But before I was able to attach gdb
to one of these processes (I wasn't prepared for that and first had to install gdb
:-() the "still running but not consuming CPU time" globus-gridftp-server
(ggs) processes were gone.
But this was for a ggs that had this PR here applied unfortunately. :-(
I didn't yet manage to create a similar situation with the ggs at da14279.
I would like to do some testing between two VMs located on two different hosts before approving the mutex (un)locking change. But this will take some time.
f170e00
to
5cfcd1f
Compare
Once again, great work Mattias (@ellert). I'm glad we have you in the GridCF. :-) Give me until evening to do my testing and then let's merge this. I'll add an approving review comment until then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this!
Thanks & congrats, guys! |
This PR addresses the following compiler and doxygen warnings:
globus-authz
globus-common
doxygen
globus-ftp-client
doxygen
other fixes
globus-ftp-control
doxygen
globus-gass-copy
doxygen
globus-gass-server-ez
globus-gass-transfer
doxygen
globus-gatekeeper
globus-gram-client
doxygen
globus-gram-client-tools
globus-gram-job-manager
globus-gram-job-manager-fork
globus-gram-job-manager-sge
globus-gridftp-server
other fixes
globus-gridftp-server-control
globus-gsi-proxy-core
globus-gsi-sysconfig
globus-gssapi-error
doxygen
globus-scheduler-event-generator
globus-xio
doxygen
globus-xio-gridftp-driver
globus-xio-gridftp-multicast
myproxy