Bugzilla – Attachment 120693 Details for
Bug 28276
Do not fetch config ($KOHA_CONF) from memcached
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28276: Do not fetch config ($KOHA_CONF) from memcached
Bug-28276-Do-not-fetch-config-KOHACONF-from-memcac.patch (text/plain), 14.27 KB, created by
Kyle M Hall (khall)
on 2021-05-07 13:53:00 UTC
(
hide
)
Description:
Bug 28276: Do not fetch config ($KOHA_CONF) from memcached
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2021-05-07 13:53:00 UTC
Size:
14.27 KB
patch
obsolete
>From 9e4401fe316158360f367eab0f142a8979c81988 Mon Sep 17 00:00:00 2001 >From: Julian Maurice <julian.maurice@biblibre.com> >Date: Sun, 2 May 2021 13:37:38 +0200 >Subject: [PATCH] Bug 28276: Do not fetch config ($KOHA_CONF) from memcached > >memcached address and namespace are in $KOHA_CONF, so it is required to >read it before being able to access the cache. And after that, >configuration is kept in memory forever. Storing this in memcached is >useless and even counter-productive, since Koha reads both the file and >the cache > >This patch addresses this issue by removing the cache-related code from >C4::Context->new. >It means that C4::Context->new will always read the configuration file, >so this patch also replaces inappropriate calls to >C4::Context->new->config by appropriate calls to C4::Context->config > >It also fixes a bug where C4::Context->new would ignore the filepath >given in parameters if there was something in cache. > >It also removes a problematic call to Koha::Caches->get_instance. >Because this call was outside of any subroutine, it would have happened >before the initialization of $C4::Context::context (which happen in >C4::Context::import) > >Test plan: >1. Do not apply the patch yet >2. Add the following line at the beginning of Koha::Config::read_from_file > warn "read_from_file($file)"; > This will allow you to check how many times the file is read. >3. Flush memcached and restart starman >4. Check the logs, you should see "read_from_file" a bunch of times >5. Apply the patch >6. Re-add the line from step 2 >7. Flush memcached and restart starman >8. Check the logs, you should see "read_from_file" only once >9. Make sure the memcached config from $KOHA_CONF (memcached_servers, > memcached_namespace) is taken into account by checking the About page > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Auth_with_ldap.pm | 1 - > C4/Context.pm | 23 +++++++---------------- > Koha/Cache.pm | 10 ++++------ > Koha/Config.pm | 2 +- > Koha/Database.pm | 24 +++++++++++------------- > about.pl | 5 ++--- > admin/systempreferences.pl | 3 +-- > misc/translator/LangInstaller.pm | 25 +++++++++++-------------- > 8 files changed, 37 insertions(+), 56 deletions(-) > >diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm >index 36b9071426..4648d3c093 100644 >--- a/C4/Auth_with_ldap.pm >+++ b/C4/Auth_with_ldap.pm >@@ -50,7 +50,6 @@ sub ldapserver_error { > } > > use vars qw($mapping @ldaphosts $base $ldapname $ldappassword); >-my $context = C4::Context->new() or die 'C4::Context->new failed'; > my $ldap = C4::Context->config("ldapserver") or die 'No "ldapserver" in server hash from KOHA_CONF: ' . $ENV{KOHA_CONF}; > my $prefhost = $ldap->{hostname} or die ldapserver_error('hostname'); > my $base = $ldap->{base} or die ldapserver_error('base'); >diff --git a/C4/Context.pm b/C4/Context.pm >index 1ab0467eca..c0e518a8bc 100644 >--- a/C4/Context.pm >+++ b/C4/Context.pm >@@ -177,7 +177,6 @@ that, use C<&set_context>. > sub new { > my $class = shift; > my $conf_fname = shift; # Config file to load >- my $self = {}; > > # check that the specified config file exists and is not empty > undef $conf_fname unless >@@ -191,18 +190,7 @@ sub new { > } > } > >- my $conf_cache = Koha::Caches->get_instance('config'); >- if ( $conf_cache->cache ) { >- $self = $conf_cache->get_from_cache('koha_conf'); >- } >- unless ( $self and %$self ) { >- $self = Koha::Config->read_from_file($conf_fname); >- if ( $conf_cache->memcached_cache ) { >- # FIXME it may be better to use the memcached servers from the config file >- # to cache it >- $conf_cache->set_in_cache('koha_conf', $self) >- } >- } >+ my $self = Koha::Config->read_from_file($conf_fname); > unless ( exists $self->{config} or defined $self->{config} ) { > warn "The config file ($conf_fname) has not been parsed correctly"; > return; >@@ -311,7 +299,7 @@ C<C4::Config-E<gt>new> will not return it. > sub _common_config { > my $var = shift; > my $term = shift; >- return if !defined($context->{$term}); >+ return unless defined $context and defined $context->{$term}; > # Presumably $self->{$term} might be > # undefined if the config file given to &new > # didn't exist, and the caller didn't bother >@@ -344,7 +332,6 @@ with this method. > > =cut > >-my $syspref_cache = Koha::Caches->get_instance('syspref'); > my $use_syspref_cache = 1; > sub preference { > my $self = shift; >@@ -356,7 +343,7 @@ sub preference { > $var = lc $var; > > if ($use_syspref_cache) { >- $syspref_cache = Koha::Caches->get_instance('syspref') unless $syspref_cache; >+ my $syspref_cache = Koha::Caches->get_instance('syspref'); > my $cached_var = $syspref_cache->get_from_cache("syspref_$var"); > return $cached_var if defined $cached_var; > } >@@ -366,6 +353,7 @@ sub preference { > my $value = $syspref ? $syspref->value() : undef; > > if ( $use_syspref_cache ) { >+ my $syspref_cache = Koha::Caches->get_instance('syspref'); > $syspref_cache->set_in_cache("syspref_$var", $value); > } > return $value; >@@ -434,6 +422,7 @@ will not be seen by this process. > > sub clear_syspref_cache { > return unless $use_syspref_cache; >+ my $syspref_cache = Koha::Caches->get_instance('syspref'); > $syspref_cache->flush_all; > } > >@@ -487,6 +476,7 @@ sub set_preference { > } > > if ( $use_syspref_cache ) { >+ my $syspref_cache = Koha::Caches->get_instance('syspref'); > $syspref_cache->set_in_cache( "syspref_$variable", $value ); > } > >@@ -508,6 +498,7 @@ sub delete_preference { > > if ( Koha::Config::SysPrefs->find( $var )->delete ) { > if ( $use_syspref_cache ) { >+ my $syspref_cache = Koha::Caches->get_instance('syspref'); > $syspref_cache->clear_from_cache("syspref_$var"); > } > >diff --git a/Koha/Cache.pm b/Koha/Cache.pm >index 54fe72f81f..c22fd4efae 100644 >--- a/Koha/Cache.pm >+++ b/Koha/Cache.pm >@@ -45,6 +45,7 @@ use Module::Load::Conditional qw(can_load); > use Sereal::Encoder; > use Sereal::Decoder; > >+use C4::Context; > use Koha::Cache::Object; > use Koha::Config; > >@@ -78,12 +79,9 @@ sub new { > # Should we continue to support MEMCACHED ENV vars? > $self->{'namespace'} ||= $ENV{MEMCACHED_NAMESPACE}; > my @servers = split /,/, $ENV{MEMCACHED_SERVERS} || ''; >- unless ( $self->{namespace} and @servers ) { >- my $koha_config = Koha::Config->read_from_file( Koha::Config->guess_koha_conf() ); >- $self->{namespace} ||= $koha_config->{config}{memcached_namespace} || 'koha'; >- @servers = split /,/, $koha_config->{config}{memcached_servers} // '' >- unless @servers; >- } >+ $self->{namespace} ||= C4::Context->config('memcached_namespace') || 'koha'; >+ @servers = split /,/, C4::Context->config('memcached_servers') // '' >+ unless @servers; > $self->{namespace} .= ":$subnamespace:"; > > if ( $self->{'default_type'} eq 'memcached' >diff --git a/Koha/Config.pm b/Koha/Config.pm >index 3e46e2728d..face2fc11f 100644 >--- a/Koha/Config.pm >+++ b/Koha/Config.pm >@@ -30,7 +30,7 @@ use constant CONFIG_FNAME => "/etc/koha/koha-conf.xml"; > # developers should set the KOHA_CONF environment variable > my $INSTALLED_CONFIG_FNAME = '__KOHA_CONF_DIR__/koha-conf.xml'; > >-# Should not be called outside of C4::Context or Koha::Cache >+# Should not be called outside of C4::Context > # use C4::Context->config instead > sub read_from_file { > my ( $class, $file ) = @_; >diff --git a/Koha/Database.pm b/Koha/Database.pm >index 0f398c7022..df4d45ce7b 100644 >--- a/Koha/Database.pm >+++ b/Koha/Database.pm >@@ -49,21 +49,19 @@ sub _new_schema { > > require Koha::Schema; > >- my $context = C4::Context->new(); >- >- my $db_driver = $context->{db_driver}; >- >- my $db_name = $context->config("database"); >- my $db_host = $context->config("hostname"); >- my $db_port = $context->config("port") || ''; >- my $db_user = $context->config("user"); >- my $db_passwd = $context->config("pass"); >- my $tls = $context->config("tls"); >+ my $db_driver = C4::Context::db_scheme2dbi(C4::Context->config('db_scheme'));; >+ >+ my $db_name = C4::Context->config("database"); >+ my $db_host = C4::Context->config("hostname"); >+ my $db_port = C4::Context->config("port") || ''; >+ my $db_user = C4::Context->config("user"); >+ my $db_passwd = C4::Context->config("pass"); >+ my $tls = C4::Context->config("tls"); > my $tls_options; > if( $tls && $tls eq 'yes' ) { >- my $ca = $context->config('ca'); >- my $cert = $context->config('cert'); >- my $key = $context->config('key'); >+ my $ca = C4::Context->config('ca'); >+ my $cert = C4::Context->config('cert'); >+ my $key = C4::Context->config('key'); > $tls_options = ";mysql_ssl=1;mysql_ssl_client_key=".$key.";mysql_ssl_client_cert=".$cert.";mysql_ssl_ca_file=".$ca; > } > >diff --git a/about.pl b/about.pl >index bfe39dbd73..344790c4b8 100755 >--- a/about.pl >+++ b/about.pl >@@ -221,8 +221,6 @@ my $warnNoActiveCurrency = (! defined Koha::Acquisition::Currencies->get_active) > > my @xml_config_warnings; > >-my $context = C4::Context->new; >- > if ( C4::Context->config('zebra_bib_index_mode') > and C4::Context->config('zebra_bib_index_mode') eq 'grs1' ) > { >@@ -235,9 +233,10 @@ if ( C4::Context->config('zebra_auth_index_mode') > push @xml_config_warnings, { error => 'zebra_auth_index_mode_is_grs1' }; > } > >+my $authorityserver = C4::Context->zebraconfig('authorityserver'); > if( ( C4::Context->config('zebra_auth_index_mode') > and C4::Context->config('zebra_auth_index_mode') eq 'dom' ) >- && ( $context->{'server'}->{'authorityserver'}->{'config'} !~ /zebra-authorities-dom.cfg/ ) ) >+ && ( $authorityserver->{config} !~ /zebra-authorities-dom.cfg/ ) ) > { > push @xml_config_warnings, { > error => 'zebra_auth_index_mode_mismatch_warn' >diff --git a/admin/systempreferences.pl b/admin/systempreferences.pl >index 7d2e1936b2..160b6fdbaa 100755 >--- a/admin/systempreferences.pl >+++ b/admin/systempreferences.pl >@@ -384,8 +384,7 @@ output_html_with_http_headers $input, $cookie, $template->output; > # .pref files. > > sub get_prefs_from_files { >- my $context = C4::Context->new(); >- my $path_pref_en = $context->config('intrahtdocs') . >+ my $path_pref_en = C4::Context->config('intrahtdocs') . > '/prog/en/modules/admin/preferences'; > # Get all .pref file names > opendir ( my $fh, $path_pref_en ); >diff --git a/misc/translator/LangInstaller.pm b/misc/translator/LangInstaller.pm >index 03e4195f22..9636758ad8 100644 >--- a/misc/translator/LangInstaller.pm >+++ b/misc/translator/LangInstaller.pm >@@ -34,7 +34,7 @@ sub set_lang { > my ($self, $lang) = @_; > > $self->{lang} = $lang; >- $self->{po_path_lang} = $self->{context}->config('intrahtdocs') . >+ $self->{po_path_lang} = C4::Context->config('intrahtdocs') . > "/prog/$lang/modules/admin/preferences"; > } > >@@ -43,9 +43,7 @@ sub new { > > my $self = { }; > >- my $context = C4::Context->new(); >- $self->{context} = $context; >- $self->{path_pref_en} = $context->config('intrahtdocs') . >+ $self->{path_pref_en} = C4::Context->config('intrahtdocs') . > '/prog/en/modules/admin/preferences'; > set_lang( $self, $lang ) if $lang; > $self->{pref_only} = $pref_only; >@@ -76,17 +74,17 @@ sub new { > $self->{langs} = \@langs; > > # Map for both interfaces opac/intranet >- my $opachtdocs = $context->config('opachtdocs'); >+ my $opachtdocs = C4::Context->config('opachtdocs'); > $self->{interface} = [ > { > name => 'Intranet prog UI', >- dir => $context->config('intrahtdocs') . '/prog', >+ dir => C4::Context->config('intrahtdocs') . '/prog', > suffix => '-staff-prog.po', > }, > ]; > > # OPAC themes >- opendir my $dh, $context->config('opachtdocs'); >+ opendir my $dh, C4::Context->config('opachtdocs'); > for my $theme ( grep { not /^\.|lib|xslt/ } readdir($dh) ) { > push @{$self->{interface}}, { > name => "OPAC $theme", >@@ -98,8 +96,8 @@ sub new { > # MARC flavours (hardcoded list) > for ( "MARC21", "UNIMARC", "NORMARC" ) { > # search for strings on staff & opac marc files >- my $dirs = $context->config('intrahtdocs') . '/prog'; >- opendir $fh, $context->config('opachtdocs'); >+ my $dirs = C4::Context->config('intrahtdocs') . '/prog'; >+ opendir $fh, C4::Context->config('opachtdocs'); > for ( grep { not /^\.|\.\.|lib$|xslt/ } readdir($fh) ) { > $dirs .= ' ' . "$opachtdocs/$_"; > } >@@ -140,7 +138,6 @@ sub po_filename { > my $self = shift; > my $suffix = shift; > >- my $context = C4::Context->new; > my $trans_path = $Bin . '/po'; > my $trans_file = "$trans_path/" . $self->{lang} . $suffix; > return $trans_file; >@@ -377,8 +374,8 @@ sub install_installer { > my $self = shift; > return unless ( $self->{installer} ); > >- my $intradir = $self->{context}->config('intranetdir'); >- my $db_scheme = $self->{context}->config('db_scheme'); >+ my $intradir = C4::Context->config('intranetdir'); >+ my $db_scheme = C4::Context->config('db_scheme'); > my $langdir = "$intradir/installer/data/$db_scheme/$self->{lang}"; > if ( -d $langdir ) { > say "$self->{lang} installer dir $langdir already exists.\nDelete it if you want to recreate it." if $self->{verbose}; >@@ -440,13 +437,13 @@ sub install_messages { > system "$self->{msgfmt} -o $mofile $pofile"; > > my $js_locale_data = 'var json_locale_data = {"Koha":' . `$self->{po2json} $js_pofile` . '};'; >- my $progdir = $self->{context}->config('intrahtdocs') . '/prog'; >+ my $progdir = C4::Context->config('intrahtdocs') . '/prog'; > mkdir "$progdir/$self->{lang}/js"; > open my $fh, '>', "$progdir/$self->{lang}/js/locale_data.js"; > print $fh $js_locale_data; > close $fh; > >- my $opachtdocs = $self->{context}->config('opachtdocs'); >+ my $opachtdocs = C4::Context->config('opachtdocs'); > opendir(my $dh, $opachtdocs); > for my $theme ( grep { not /^\.|lib|xslt/ } readdir($dh) ) { > mkdir "$opachtdocs/$theme/$self->{lang}/js"; >-- >2.24.3 (Apple Git-128)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28276
:
120352
|
120428
| 120693