On 18/05/11 13:10, Shree wrote:
I don't think we can help you because we don't know what HandleIds() does. I
note it may return -1 and causes all of the remaining ids in your batch to be
skipped. Some further comments below:
use strict;
use warnings;
my $dsn = 'dbi:Oracle:dbname';
my $user = 'xxxxx';
my $pass = 'xxxxxxxxxxx';
my $attr = {
FetchHashKeyName => 'NAME_lc',
RaiseError => 1,
PrintError => 0,
AutoCommit => 0,
ChopBlanks => 1,
};
my $rowcount = 11;
my $commitLevel = 10;
my $dbh = DBI->connect($dsn, $user, $pass, $attr);
my $queued_recs = $dbh->prepare("SELECT id from (SELECT id FROM queue
WHERE processed=\'F\' ORDER BY id ASC) where ROWNUM< $rowcount");
You could prepare your update statement here and avoid calling prepare
repeatedly.
my $going = 1;
while($going) {
$queued_recs->execute();
my @ids2process;
while ( my @ids = $queued_recs->fetchrow_array ) {
push(@ids2process, $ids[0]);
}
You know you are only going to get a list of ids back here so why bother
fetchrow_arraying it only to push onto @ids2process. Why not just
fetchall_arrayref in one go then loop over those.
if(!scalar(@ids2process)) {
sleep(5);
}
if(scalar(@ids2process)> 0 ) {
my $count=0;
foreach my $ids (@ids2process){
$count++;
my $ret = HandleIds($id);
next if($ret == -1);
We don't know what HandleIds does and it causes all remaining ids to be
ignored. This is probably where your problem is.
my $updated = '';
eval{
my $update_ids = $dbh->prepare("UPDATE
queue set processed = \'T
\' where id = $ids");
$updated = $update_ids->execute();
};
A good habit to get into is to use placeholders in your SQL - there are a lot
of good reasons for this documented all over the place:
prepare the update outside the loop with:
my $update_ids = $dbh->prepare(q/update queue set processed = ? where id = ?/);
then
$update_ids->execute('T', $ids);
if($@) { $log->error("Unable to update record for - id:
$id,
updated: $updated");}
else { $log->info("Updated record for - id: $id,
updated:
$updated");}
What if your code is wrong and you pass an id which does not exist in the table
- you do not know the update changed nothing. Check $updated contains 1.
if(($count % $commitLevel == 0) || ($count ==
scalar(@ids2process)))
{
my $commited = $dbh->commit;
$log->info("COMMIT at $count Ids, commited:
$commited");
}
}
There is something wrong with the above logic due to HandleIds() again. If your
select returns 10 rows but HandleIds returns -1 for the last id, next causes
the loop to end and your commit did not get called. Much better to do:
Personally I don't like turning off AutoCommit like this I prefer to do:
$dbh->begin_work;
do my work;
$dbh->commit;
i.e., put the commit outside the loop then you don't car how many ids were
processed, you just commit all the updates.
}else {
sleep(5);
}
}
Did you intend to sleep for 10s when there are no ids or 5?
Martin
--
Martin J. Evans
Easysoft Limited
http://www.easysoft.com
