[torquedev] Reorganizing TORQUE Source Code
glen.beane at gmail.com
Tue Feb 24 16:34:51 MST 2009
Hi Josh, see my comments below.
On Tue, Feb 24, 2009 at 5:19 PM, Josh Butikofer
<josh at clusterresources.com> wrote:
> TORQUE Developers:
> Over the past few months I've been looking through the TORQUE source code
> and trying to find ways to improve its organization. I've found that there
> are several pieces of code that are data structures and utility functions
> used in both pbs_server and pbs_mom that are NOT part of the shared library
> package. This is fine, except for the fact that in some cases the
> functions/data structures are defined twice--once in pbs_mom code and once
> in pbs_server code. Oftentimes it is a direct copy and paste. In my mind
> this is not only silly, but potentially dangerous, as keeping these two
> copies in sync is a maintenance burden.
I agree cut and paste code that appears in pbs_mom and pbs_server is
not "A Good Thing"(tm). TORQUE code can be difficult to understand
and maintain, and things like cut and paste code make maintenance more
difficult and are a potential source of bugs when the code is modified
and not kept in sync. Any thing that makes the code easier to
maintain and removes a potential source of bugs is a good thing.
> I've also found that I want to add a generic function to TORQUE, but am
> forced to add it to some unrelated module like "job_func.c"...even though it
> has NOTHING to do with jobs, per se.
If the utility function is not static and has the potential to be used
in several places in torque, then I agree placing it in something like
job_func.c just because you don't have any where else to put it and
job_func.c is the first place you need to use the function (but it may
have wider applicability) is not ideal.
> So I have a proposal.
> We would like to create a new "utility" module in TORQUE. This C module
> would be statically linked into both the pbs_server and pbs_mom at compile
> time. It would NOT be part of the shared library, but it would still be
> found in its own "Libutil" directory. We would move any functions
> defined/used by both pbs_server and pbs_mom into this module, as well as any
> other functions that are generic enough that it would warrant their
> placement there.
> We already have a patch to do this work and are eager to check it in. I just
> wanted to double check if there are any objections or if anyone has any
> reservations. We plan on introducing this reorganized code into the next
> version of TORQUE 2.3.x. (No code logic is actually changing, so the
> functionality should remain exactly the same.)
> Let me know what you think.
I would just mention that we have been bitten by bugs introduced by
code refactoring that seemed harmless. In some cases the old TORQUE
code inherited from OpenPBS is fragile and might depend on side
effects, etc. Hopefully we are moving away from this state. I would
just urge thorough testing in CRI's lab and by all TORQUE devs.
More information about the torquedev