From 82af5cf5181e007d38bac2693e85a2d7680971fc Mon Sep 17 00:00:00 2001 From: Rene Krenn Date: Mon, 3 Mar 2025 18:41:30 +0100 Subject: [PATCH] MT#62283 "for update no lock" logic for GET /subscribers and /contracts to unblock the web UI on heavy loaded systems with call rating enabled, "select .. for update no lock" will no longer wait for acquiring locks when retrieving subscriber or contract contract pages. the contract_balance record creation is will be skipped for such contracts that are locked elsewhere. for rails such as /customerbalances, an explicit contract id row lock timeout is introduced, so it does not depend on the mariadb wait_timeout. Change-Id: I6e96342f3f761279a5876c871d2477977823a39e --- .../Panel/Controller/API/BalanceIntervals.pm | 7 ++- lib/NGCP/Panel/Controller/API/Contracts.pm | 7 ++- .../Panel/Controller/API/CustomerBalances.pm | 7 ++- lib/NGCP/Panel/Controller/API/Customers.pm | 7 ++- .../Controller/API/SubscriberPreferences.pm | 7 ++- lib/NGCP/Panel/Controller/API/Subscribers.pm | 7 ++- lib/NGCP/Panel/Controller/Customer.pm | 4 +- lib/NGCP/Panel/Controller/Subscriber.pm | 2 +- lib/NGCP/Panel/Role/API/Subscribers.pm | 2 +- lib/NGCP/Panel/Utils/Contract.pm | 57 +++++++++++++++++-- lib/NGCP/Panel/Utils/ProfilePackages.pm | 11 ++-- lib/NGCP/Panel/Utils/ProvisioningTemplates.pm | 2 +- lib/NGCP/Panel/Utils/Subscriber.pm | 2 +- 13 files changed, 95 insertions(+), 27 deletions(-) diff --git a/lib/NGCP/Panel/Controller/API/BalanceIntervals.pm b/lib/NGCP/Panel/Controller/API/BalanceIntervals.pm index 953967c31a..2480a75a0b 100644 --- a/lib/NGCP/Panel/Controller/API/BalanceIntervals.pm +++ b/lib/NGCP/Panel/Controller/API/BalanceIntervals.pm @@ -95,9 +95,12 @@ sub GET :Allow { my $now = NGCP::Panel::Utils::DateTime::current_local; my $contracts_rs = $self->item_rs($c,0,$now); (my $total_count, $contracts_rs, my $contracts_rows) = $self->paginate_order_collection($c, $contracts_rs); - my $contracts = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, + my $contracts = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( + c => $c, rs => $contracts_rs, - contract_id_field => 'id'); + contract_id_field => 'id', + #skip_locked => 1, + ); my (@embedded, @links); my $form = $self->get_form($c); $self->expand_prepare_collection($c); diff --git a/lib/NGCP/Panel/Controller/API/Contracts.pm b/lib/NGCP/Panel/Controller/API/Contracts.pm index 1ca573fd5b..0ce07d4524 100644 --- a/lib/NGCP/Panel/Controller/API/Contracts.pm +++ b/lib/NGCP/Panel/Controller/API/Contracts.pm @@ -94,9 +94,12 @@ sub GET :Allow { my $now = NGCP::Panel::Utils::DateTime::current_local; my $contracts_rs = $self->item_rs($c,0,$now); (my $total_count, $contracts_rs, my $contracts_rows) = $self->paginate_order_collection($c, $contracts_rs); - my $contracts = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, + my $contracts = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( + c => $c, rs => $contracts_rs, - contract_id_field => 'id'); + contract_id_field => 'id', + skip_locked => 1, + ); my (@embedded, @links); my $form = $self->get_form($c); $self->expand_prepare_collection($c); diff --git a/lib/NGCP/Panel/Controller/API/CustomerBalances.pm b/lib/NGCP/Panel/Controller/API/CustomerBalances.pm index f91b9c5c4a..957cb30d63 100644 --- a/lib/NGCP/Panel/Controller/API/CustomerBalances.pm +++ b/lib/NGCP/Panel/Controller/API/CustomerBalances.pm @@ -125,9 +125,12 @@ sub GET :Allow { my $now = NGCP::Panel::Utils::DateTime::current_local; my $items_rs = $self->item_rs($c,0,$now); (my $total_count, $items_rs, my $items_rows) = $self->paginate_order_collection($c, $items_rs); - my $items = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, + my $items = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( + c => $c, rs => $items_rs, - contract_id_field => 'id'); + contract_id_field => 'id', + #skip_locked => 1, + ); my (@embedded, @links); my $form = $self->get_form($c); $self->expand_prepare_collection($c); diff --git a/lib/NGCP/Panel/Controller/API/Customers.pm b/lib/NGCP/Panel/Controller/API/Customers.pm index e384d04daf..c4607b4525 100644 --- a/lib/NGCP/Panel/Controller/API/Customers.pm +++ b/lib/NGCP/Panel/Controller/API/Customers.pm @@ -167,9 +167,12 @@ sub GET :Allow { my $now = NGCP::Panel::Utils::DateTime::current_local; my $customers_rs = $self->item_rs($c,$now); (my $total_count, $customers_rs, my $customers_rows) = $self->paginate_order_collection($c, $customers_rs); - my $customers = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, + my $customers = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( + c => $c, rs => $customers_rs, - contract_id_field => 'id'); + contract_id_field => 'id', + skip_locked => 1, + ); my (@embedded, @links); my $form = $self->get_form($c); $self->expand_prepare_collection($c); diff --git a/lib/NGCP/Panel/Controller/API/SubscriberPreferences.pm b/lib/NGCP/Panel/Controller/API/SubscriberPreferences.pm index 8fcfd5b86e..7bda0668df 100644 --- a/lib/NGCP/Panel/Controller/API/SubscriberPreferences.pm +++ b/lib/NGCP/Panel/Controller/API/SubscriberPreferences.pm @@ -92,9 +92,12 @@ sub GET :Allow { { my $subscribers_rs = $self->item_rs($c, "subscribers"); (my $total_count, $subscribers_rs, my $subscribers_rows) = $self->paginate_order_collection($c, $subscribers_rs); - my $subscribers = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, + my $subscribers = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( + c => $c, rs => $subscribers_rs, - contract_id_field => 'contract_id'); + contract_id_field => 'contract_id', + skip_locked => 1, + ); my $now = NGCP::Panel::Utils::DateTime::current_local; my (@embedded, @links, %contract_map); $self->expand_prepare_collection($c); diff --git a/lib/NGCP/Panel/Controller/API/Subscribers.pm b/lib/NGCP/Panel/Controller/API/Subscribers.pm index 31edcf3720..8d695b5f51 100644 --- a/lib/NGCP/Panel/Controller/API/Subscribers.pm +++ b/lib/NGCP/Panel/Controller/API/Subscribers.pm @@ -319,9 +319,12 @@ sub GET :Allow { { my $subscribers_rs = $self->item_rs($c); (my $total_count, $subscribers_rs, my $subscribers_rows) = $self->paginate_order_collection($c, $subscribers_rs); - my $subscribers = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, + my $subscribers = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( + c => $c, rs => $subscribers_rs, - contract_id_field => 'contract_id'); + contract_id_field => 'contract_id', + skip_locked => 1, + ); my $now = NGCP::Panel::Utils::DateTime::current_local; my (@embedded, @links, %contract_map); my ($form) = $self->get_form($c); diff --git a/lib/NGCP/Panel/Controller/Customer.pm b/lib/NGCP/Panel/Controller/Customer.pm index 1b5682b34a..e81c061c24 100644 --- a/lib/NGCP/Panel/Controller/Customer.pm +++ b/lib/NGCP/Panel/Controller/Customer.pm @@ -886,7 +886,7 @@ sub subscriber_create :Chained('base') :PathPart('subscriber/create') :Args(0) { $schema->txn_do(sub { NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( - schema => $schema, contract_id => $c->stash->{contract}->id) if $c->stash->{contract}; + c => $c, schema => $schema, contract_id => $c->stash->{contract}->id) if $c->stash->{contract}; my $preferences = {}; my $pbx_group_ids = []; @@ -1504,7 +1504,7 @@ sub pbx_group_create :Chained('base') :PathPart('pbx/group/create') :Args(0) { $schema->txn_do( sub { NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( - schema => $schema, contract_id => $c->stash->{contract}->id) if $c->stash->{contract}; + c => $c, schema => $schema, contract_id => $c->stash->{contract}->id) if $c->stash->{contract}; my $preferences = {}; diff --git a/lib/NGCP/Panel/Controller/Subscriber.pm b/lib/NGCP/Panel/Controller/Subscriber.pm index 8b05245b74..9a47bc749d 100644 --- a/lib/NGCP/Panel/Controller/Subscriber.pm +++ b/lib/NGCP/Panel/Controller/Subscriber.pm @@ -2968,7 +2968,7 @@ sub edit_master :Chained('master') :PathPart('edit') :Args(0) :Does(ACL) :ACLDet $schema->txn_do(sub { NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( - schema => $schema, contract_id => $subscriber->contract->id); + c => $c, schema => $schema, contract_id => $subscriber->contract->id); my $email = delete $form->params->{email} || undef; my $timezone = delete $form->values->{timezone}{name} || undef; diff --git a/lib/NGCP/Panel/Role/API/Subscribers.pm b/lib/NGCP/Panel/Role/API/Subscribers.pm index 5f91857fc9..ba9706e8cf 100644 --- a/lib/NGCP/Panel/Role/API/Subscribers.pm +++ b/lib/NGCP/Panel/Role/API/Subscribers.pm @@ -396,7 +396,7 @@ sub prepare_resource { my ($cid) = @_; my $contract = $self->get_customer($c, $cid); NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( - schema => $c->model('DB'), contract_id => $contract->id) if $contract; + c => $c, schema => $c->model('DB'), contract_id => $contract->id) if $contract; return $contract; }, ); diff --git a/lib/NGCP/Panel/Utils/Contract.pm b/lib/NGCP/Panel/Utils/Contract.pm index fc56dcec87..713347cd65 100644 --- a/lib/NGCP/Panel/Utils/Contract.pm +++ b/lib/NGCP/Panel/Utils/Contract.pm @@ -7,6 +7,25 @@ use DBIx::Class::Exception; use NGCP::Panel::Utils::DateTime; use NGCP::Panel::Utils::CallList qw(); +my $lock_timeout = 5; + +use DBIx::Class::SQLMaker::MySQL qw(); +{ + no warnings 'redefine'; ## no critic (ProhibitNoWarnings) + *DBIx::Class::SQLMaker::MySQL::_lock_select = sub { + my ($self, $type) = @_; + + my $sql; + if (ref($type) eq 'SCALAR') { + $sql = "FOR $$type"; + } else { + $sql = $DBIx::Class::SQLMaker::MySQL::for_syntax->{$type} || $self->throw_exception( "Unknown SELECT .. FOR type '$type' requested" ); + } + + return " $sql"; + }; +} + sub recursively_lock_contract { my %params = @_; @@ -465,7 +484,20 @@ sub is_reseller_product { sub acquire_contract_rowlocks { my %params = @_; - my($c,$schema,$rs,$contract_id_field,$contract_ids,$contract_id) = @params{qw/c schema rs contract_id_field contract_ids contract_id/}; + my($c, + $schema, + $rs, + $contract_id_field, + $contract_ids, + $contract_id, + $skip_locked) = @params{qw/c + schema + rs + contract_id_field + contract_ids + contract_id + skip_locked + /}; $schema //= $c->model('DB'); @@ -491,9 +523,14 @@ sub acquire_contract_rowlocks { $c->log->debug('contract ID to be locked: ' . $contract_id) if $c; my $contract = $schema->resultset('contracts')->find({ id => $contract_id - },{for => 'update'}); + },{ for => ($skip_locked ? \"update skip locked" : \"update wait $lock_timeout") }); $t2 = time; - $c->log->debug('contract ID ' . $contract_id . ' locked (' . ($t2 - $t1) . ' secs)') if $c; + if ($c) { + $c->log->debug('contract ID ' . $contract_id . ' locked (' . ($t2 - $t1) . ' secs)'); + $c->stash->{catchup_contract_ids} = { + $contract_id => (defined $contract ? 1 : 0), + } if $skip_locked; + } return $contract; } elsif ((scalar @contract_ids_to_lock) > 0) { @contract_ids_to_lock = sort { $a <=> $b } @contract_ids_to_lock; #"Access your tables and rows in a fixed order." @@ -501,9 +538,19 @@ sub acquire_contract_rowlocks { $c->log->debug('contract IDs to be locked: ' . $contract_ids_label) if $c; my @contracts = $schema->resultset('contracts')->search({ id => { -in => [ @contract_ids_to_lock ] } - },{for => 'update'})->all; + },{ for => ($skip_locked ? \"update skip locked" : \"update wait $lock_timeout") })->all; $t2 = time; - $c->log->debug('contract IDs ' . $contract_ids_label . ' locked (' . ($t2 - $t1) . ' secs)') if $c; + if ($c) { + $c->log->debug('contract IDs ' . $contract_ids_label . ' locked (' . ($t2 - $t1) . ' secs)'); + if ($skip_locked) { + my %cids = map { $_->id => undef; } @contracts; + foreach (@contract_ids_to_lock) { + $c->stash->{catchup_contract_ids} = { + $_ => (exists $cids{$_} ? 1 : 0), + }; + } + } + } if (defined $contract_ids || defined $contract_id) { return [ @contracts ]; } else { diff --git a/lib/NGCP/Panel/Utils/ProfilePackages.pm b/lib/NGCP/Panel/Utils/ProfilePackages.pm index 8a059b0d1c..5d1a31185a 100644 --- a/lib/NGCP/Panel/Utils/ProfilePackages.pm +++ b/lib/NGCP/Panel/Utils/ProfilePackages.pm @@ -77,7 +77,7 @@ sub resize_actual_contract_balance { my($c,$contract,$old_package,$actual_balance,$is_topup,$topup_amount,$now,$schema,$profiles_added) = @params{qw/c contract old_package balance is_topup topup_amount now schema profiles_added/}; $schema //= $c->model('DB'); - $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(schema => $schema, contract_id => $contract->id); + $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, schema => $schema, contract_id => $contract->id); $is_topup //= 0; $topup_amount //= 0.0; $profiles_added //= 0; @@ -217,9 +217,12 @@ sub catchup_contract_balances { my($c,$contract,$old_package,$now,$suppress_underrun,$is_create_next,$last_notopup_discard_intervals,$last_carry_over_mode,$topup_amount,$profiles_added) = @params{qw/c contract old_package now suppress_underrun is_create_next last_notopup_discard_intervals last_carry_over_mode topup_amount profiles_added/}; return unless $contract; + if ($c->stash->{catchup_contract_ids}) { + return unless $c->stash->{catchup_contract_ids}->{$contract->id}; + } my $schema = $c->model('DB'); - $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(schema => $schema, contract_id => $contract->id); + $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, schema => $schema, contract_id => $contract->id); $now //= NGCP::Panel::Utils::DateTime::set_local_tz($contract->modify_timestamp); $old_package = $contract->profile_package if !exists $params{old_package}; my $contract_create = NGCP::Panel::Utils::DateTime::set_local_tz($contract->create_timestamp // $contract->modify_timestamp); @@ -442,7 +445,7 @@ sub topup_contract_balance { my($c,$contract,$package,$voucher,$amount,$now,$request_token,$schema,$log_vals,$subscriber) = @params{qw/c contract package voucher amount now request_token schema log_vals subscriber/}; $schema //= $c->model('DB'); - $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(schema => $schema, contract_id => $contract->id); + $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, schema => $schema, contract_id => $contract->id); $now //= NGCP::Panel::Utils::DateTime::current_local; my $voucher_package = ($voucher ? $voucher->profile_package : $package); @@ -587,7 +590,7 @@ sub create_initial_contract_balances { my($c,$contract,$now) = @params{qw/c contract now/}; my $schema = $c->model('DB'); - $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(schema => $schema, contract_id => $contract->id); + $contract = NGCP::Panel::Utils::Contract::acquire_contract_rowlocks(c => $c, schema => $schema, contract_id => $contract->id); $now //= NGCP::Panel::Utils::DateTime::set_local_tz($contract->create_timestamp // $contract->modify_timestamp); my ($start_mode,$interval_unit,$interval_value,$initial_balance,$underrun_profile_threshold,$underrun_lock_threshold); diff --git a/lib/NGCP/Panel/Utils/ProvisioningTemplates.pm b/lib/NGCP/Panel/Utils/ProvisioningTemplates.pm index 3ea25d1122..848a1eb26f 100644 --- a/lib/NGCP/Panel/Utils/ProvisioningTemplates.pm +++ b/lib/NGCP/Panel/Utils/ProvisioningTemplates.pm @@ -967,7 +967,7 @@ sub _init_subscriber_context { my ($cid) = @_; my $contract = $schema->resultset('contracts')->find($cid); NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( - schema => $schema, contract_id => $contract->id) if $contract; + c => $c, schema => $schema, contract_id => $contract->id) if $contract; return $contract; }, item => $item, diff --git a/lib/NGCP/Panel/Utils/Subscriber.pm b/lib/NGCP/Panel/Utils/Subscriber.pm index 0222177fe8..e2cb4c400e 100644 --- a/lib/NGCP/Panel/Utils/Subscriber.pm +++ b/lib/NGCP/Panel/Utils/Subscriber.pm @@ -1796,7 +1796,7 @@ sub terminate { $schema->txn_do(sub { NGCP::Panel::Utils::Contract::acquire_contract_rowlocks( - schema => $schema, contract_id => $subscriber->contract->id); + c => $c, schema => $schema, contract_id => $subscriber->contract->id); my $prov_subscriber = $subscriber->provisioning_voip_subscriber;