Talk:Native Code Packaging Changes
Jeff G's comments
Regarding making a PL/PgSQL version of iplike: I think there are a few tweaks to that function that would speed it up, like special-casing what I imagine are the four most common rules: '*.*.*.*', 'N.*.*.*', 'N.N.*.*', and 'N.N.N.*'. Cut out a little string processing and see how it helps.
A little clarification on the above: Chris Browne in his original bug mentions the INET function and associated operators. We could take each of these cases, based on a regexp string match for the rule (or an exact string comparison for the '*.*.*.*' case), and turn e.g. IPADDR IPLIKE '172.29.*.*' into:
INET IPADDR << INET '172.29.0.0/16'
Which means "turn both of these strings into super-fast internal representations, then do super-fast bitwise math to see if the LHS is contained within the RHS".
Regarding InetAddress.isReachable(): I have suggested in the past that we investigate this possibility, but after reading DJ's comments below I agree that the behavior could be unacceptable in some not-so-edge cases. One hallmark of an enterprise-grade product in my mind is deterministic behavior. Strategizing ICMP is something we probably should do, but let's make sure that the user has to break glass and pull a clearly-marked lever in order to use an isReachable strategy if we build one.
There's a Sun RFE bug to add proper raw socket / ICMP support to Java. It's currently (04 Jun 2007) in the top 25 RFEs. Go vote for it :)
+1 switching to the Sun Workshop compiler on Solaris per DJ's suggestion
As of June 4, 2007, these comments are largely critical, but I have some suggestions that I will add once I have more time.
Things I wholeheartedly agree with
- Separate modules for native code
- Classification of the modules
- GNU autoconf for C code
Remember that we have not only RPMs, but Debian packages, Solaris packages, and we also compile on FreeBSD, as well.
Also, on Solaris things can be a little messy when compiling with GCC because the libgcc_s shared library can be installed in a number of places, depending on whether you compiled with GCC in /usr/sfw on later versions of Solaris, GCC from Sun Freeware, GCC from Blastwave, etc.. Maybe we should instead compile the packages that we ship with Sun's CC, which is now free, and doesn't place a requirement on requiring an additional shared library at runtime which might be in a number of different places.
We also have a similar problem on Solaris, and to a lesser extent on other systems, with the location of librrd.so (Sun Freeware, vs. Blastwave, vs. self-compiled). We could either compile statically, or compile dynamically and explicitly state our dependencies in the package (e.g.: we depend on version X of librrd), and if we can, include a dependency for the specific file we are expecting, as some distributions might put librrd in a different place (hmm... maybe this isn't much of a problem anymore, as it seems to go in /usr/lib or /usr/lib64).
Lastly, regarding shipping all of the native code for different instruction sets that might run on a machine in the same package (e.g.: x86 and x64): I think we should keep them in separate packages. This is how pretty much every existing package that I've seen does it, and in some cases, depending on where we want to put the library, the necessary dependencies, such as packages or parent directories that might not exist (for example, if we try to install both x86 and x64 code on a box that only has x86 stuff installed).
Some sites won't allow iplike to be installed in template1, for example, if it is a shared database server. also note that this will only work if we are creating a brand new database, as template1 is just a template that is used when new databases get created. an existing 'opennms' database will need to have the stored procedure added to it in addition to template1. is there something wrong with the current strategy?
What does the last part of this sentence mean? "Need to properly handle detecting postgresql version since the stored procedure code not has version MAGIC in it."
PL/PgSQL implementation: a good idea, but the performance is currently much worse than that of the C version (which isn't great to begin with). Could be significantly improved by using an integer field. Now, maybe the poor performance isn't a problem. Adding some timing to the JdbcFilterDao might help with getting an idea of how things are running with the PL/PgSQL implementation in place.
Regarding no dependencies: on Solaris when compiled with GCC, there is a dependency on libgcc_s. See the above comment about packaging.
Replacing our ICMP code with Java's InetAddress.isReachable
I'm against this. The operation of InetAddress.isReachable is documented to not use a specific method to reach the host in question, so we shouldn't depend on it to be doing something specific under the covers, in particular an ICMP echo request. I think it is *bad* for a network management system to act in an indeterminate way, which is what InetAddress.isReachable does when it makes a decision on reachability implementation-dependent. I wouldn't mind making the implementation pluggable, but I would *not* like to see InetAddress.isReachable used solely in discovery or anywhere, and I would very much prefer that we continue to use our ICMP code as the shipping default (or anyone else's ICMP code--the key function is that it is something we *know* will do an ICMP echo request).
Depending on the underlying implemetation of isReachable and runtime conditions (for example, running the Sun JVM on Windows or running as a non-root user on UNIX with the Sun JVM), InetAddress.isReachable will instead do a TCP check that may return false positives or false negatives depending on the setup of network and host firewalls between the NMS and the target. Using code that only uses ICMP echo requests might produce false negatives, but their likelyhood should be much less likely than a TCP-based check and the likelyhood of false positives are almost nil.