On Dec 19, 2007 1:35 PM, Florian Forster <<a href="mailto:octo@verplant.org" target="_blank">octo@verplant.org</a>> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi again :)</blockquote><div>Hi Florian !<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><br>On Tue, Dec 18, 2007 at 02:03:20PM +0100, Sebastian Harl wrote:
<br>> Just a small nitpick about coding style:<br><br></div>Sorry again if we appear picky here, but it's my firm conviction that<br>code should be easy to read, not (necessarily) easy write.<br></blockquote><div>
<br>
It's ok ... I haven't develop anything in the last years .. only my scripts, and sysadmin smalls program .... sorry ... ;-)<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>I have some questions regarding the data:<br>- What do the garbage collector statistics mean? Is `count' the number<br> of times the GC has been run? What is GC `time'? The time spend<br> collecting garbage? Why should these two values be bundled together?
</blockquote><div><br>Yes, it'is. I put it on one type to have only one rrrd file for gc type, but you can put it on separate files if you want. <br><br>About it ... in the first message I send you a collectd servlet. If you don't want stats about garbage collector then you can use the manager/status servlet that is bundled with tomcat.
<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>- What's this connector business? Possibly that's obvious to someone who
<br> knows how Tomcat works, but I'm afraid I have no idea ;) I'm afraid<br> the Tomcat documentation is a bit short in this respect ;)<br> <<a href="http://tomcat.apache.org/tomcat-5.5-doc/connectors.html" target="_blank">
http://tomcat.apache.org/tomcat-5.5-doc/connectors.html</a>></blockquote><div><br>A tomcat connector is a port & protocol method to connect to tomcat. On the tomcat instance you can define a connector on a 8080 port with HTTP procotol, another 8009 with AJP protocol, another on port 80443 with HTTPS protocol, etc .....
all stats on Mbeans are under the connector instances ... <br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>I've also made some changes to your code and put that in a separate
<br>
branch of the repository. The changed file is attached, please use it as<br>a base for further changes. My changes include:</blockquote><div><br>I read the file and all your changes are ok for me... Now is easier to read and safer ... good job !
<br><br>Only ... on the tomcat_shutdown function, I forgot change plugin_unregister_config to plugin_unregister_complex_config ....<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>- Fixed the changing indentation. There were parts indented with one<br> tab, two spaces and four spaces. I changed it to have two-space
<br> indentation everywhere.<br>- Removed the `Instance' setting from _within_ the block. This removes<br> this ambiguity:<br> <Plugin tomcat><br> <Instance "foo"><br> Instance "bar"
<br> ...<br> </Instance><br> </Plugin><br> Would that instance be named `foo' or `bar'?<br>- Removed some stacking depth with the following reformulation:<br> Instead of<br> for (...)<br>
{<br> if (foo)<br> {<br> /* lot and lot of lines, including more for-loops<br> * and if-blocks */<br> }<br> }<br> rather use<br> for (...)<br> {<br> if (!foo)<br> continue;
<br> /* the other lines, outside the if-block now */<br> }<br>- I broke up the types, like `gc' with the data sources `time' and<br> `count' is not `gc_time' and `gc_count'.<br>- I've changed all long lines so that no lines has more than 78
<br> characters.<br><br>If any of these changes broke the plugin for good, please let me know -<br>I couldn't test the code myself. Due to the changed types you will have<br>to adapt your `types.db' before testing, though.
<br><div><br>> > if (ti->url == NULL)<br>> > return (-1);<br>><br>> Is it really appropriate to return an error in this case?<br><br></div>Yes, but I changed it so the error occurs during configuration.
<br><br>So, thank you very much for the plugin Justo :) It'd be great if you<br>could test my changes and if you could double check what I put into the<br>manpage at some point (right now it's undocumented).</blockquote>
<div><br>You are welcome !! ... thanks for you and sebastian, you make a fantastic job ! ;-)<br><br>I see the web git repository and I can't find this branch to see the man page ... can you send me a copy for read it ?? ;-)
<br><br>I'm testing your version ... I hope tomorrow send you my comments ...<br><br>thanks !<br>j<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br><br>
Regards,<br>-octo<br><font color="#888888">--<br></font><div><div></div><div>Florian octo Forster<br>Hacker in training<br>GnuPG: 0x91523C3D<br><a href="http://verplant.org/" target="_blank">http://verplant.org/
</a><br></div></div><br>-----BEGIN PGP SIGNATURE-----<br>Version: GnuPG v1.4.6 (GNU/Linux)<br><br>iD8DBQFHaRAKOIGYkRnzlcMRAsh2AKCcONc9Vc3a24e+FJw/LizkYlsIVgCgvnn1<br>ZbnWNEUbVEL6su4gsMFDy1k=<br>=f47m<br>-----END PGP SIGNATURE-----
<br><br></blockquote></div><br>