I'm just going to point out a few problems.  These are not all related 
to your questions.

>package Holds;
>

The case of "Holds" doesn't match the example sub you posted above.  I'm 
assuming that was a typo.

>use strict;
>use Carp;
>use warnings;
>use QueryPrint;
>use vars qw($dbh $processed_hnd $status_hnd);
>use gentimeid; # generate time id based
>
>
>sub new {         
>            my $invocant = shift;
>            my $class = ref($invocant) || $invocant;
>

That looks like voodoo code copied from a man page.  If you call this as 
Holds->new(), you don't need that junk about ref.  (And most people 
recommend against the "new Holds" syntax.)

>            my $self  = { @_ };
>            bless ($self, $class);
>            $dbh = db_connect();
>

You don't seem to need this.  You aren't using the database handle for 
anything in this sub and you aren't gaining anything by calling it here.

>sub GetProcessed {
>
>my $self = shift;
>
>        #### This has a bug, somtimes the cached query doesn't stick around.
>

If you lose your database connection, Apache::DBI will reconnect.  Any 
prepared queries will be lost.  You *must* prepare every time, but see 
below...

>sub db_connect {
>
>require DBI;
>

You don't need that.  You should have already loaded it in startup.pl.

>my $dbname = 'CS';
>my ($dbuser, $dbpasswd) = ('myuser', 'mypass');
>

Probably should be in a config file, rather than buried in here.

>my $dbh = DBI->connect("DBI:mysql:$dbname", $dbuser, $dbpasswd)
>   or die "can't connect: $DBI::errstr\n";
>   
>   # we need these waiting for queries, so we are going to prepare them ahead of
> time, and yes
>   # horror of horror they will be global. Sorry Mom I tried :( 
>   $processed_hnd = $dbh->prepare_cached("select ord_tpak_processed from orders
>where ord_num=?") or confess("can't get tpak processed");
>   $status_hnd = $dbh->prepare_cached("select is_hold_set,holdstate from
>holds where ord_num=?") or confess("can't get hold status");
>   #DBI->trace(2,"/usr/local/apache/htdocs/out.log");
>   return $dbh;
>

Don't put those in globals.  The prepare_cached call already stores them 
for the life of your database connection.  Apache::DBI will keep that 
connection alive (in a global hash) as long as it can and reconnect if 
the connection is lost.  If the connection does get lost, the statement 
handles in these globals will stop working.  You do recreate them every 
time since you call this sub every time, but you could lose the 
connection between the time this sub is called and the time you use 
these handles.

> 2. Every once in a while I get an out of memory error.
>

You can control process growth over time in a number of ways.  They are 
documented in the mod_perl guide.

>3. My main search result page is getting cached, the closure type of
>problem.
>

Are you using any modules that have subs with sub ref prototypes, like 
Error.pm?  That can do it.

>All I have read says that because I am using oop
>modules and use strict along with use vars that should not happen.
>

It's actually really easy to create closures.  Here is a closure:

my $holdtype = $q->param('holdstate');
display_holdtype();

sub display_holdtype {
    print "holdtype: $holdtype in process $$\n";
}

This will always print whatever the value was the first time, no matter 
what you change it to later.  (The first time for that process, that 
is.)  Watch out for things like that.  You should always pass params 
explicitly.

>4. I know the way I have done these db connects is sloppy. But I can't seem
>to find a better way. Could I make one db_connect sub,and inherite it all
>though my modules? 
>

Make one sub that returns a database handle and use it from everywhere. 
 Doesn't need to be inherited, you can just stick it in a module that 
all the other modules call.

Hope some of that was helpful,
Perrin

Reply via email to