Deprecate db_ignore_replica() and convert it to service

Agreed here, This is a badly name function. I also updated the docs to make a bit more sense.

If we're going to mess with the function name, let's just eliminate it entirely and move that functionality to \Database. It's just about the only function in database.inc that isn't entirely vestigial at this point; let's get rid of it.

Well it should come out of a floating function. I'm not sure where else to put it. I checked and you're right; the Database class has only ignoreTarget(), which is set per-request from an event listener. I'm not sure what a proper OOP location for it would be other than another service (which maybe then subsumes the event listener, too?)

db_ignore_replica() is still the only function in database.inc that's not deprecated.

Here is a service implementation

Thinking a bit more about ... I think this could store this value inside protected variable instead of session but looks no way cos we need this value in next request for the same user.

better title

proper namespace

Failed test shows that session->isStarted() is not needed

Fix typo

+++ b/core/includes/database.inc
@@ -1032,18 +1032,13 @@ function db_change_field($table, $field, $field_new, $spec, $keys_new = []) {
+  @trigger_error('db_ignore_replica() is deprecated in Drupal 8.6.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Database\ReplicaKillSwitch::trigger(). See tbd.', E_USER_DEPRECATED);

needs CR link

In #2848820-16: Replace all calls to db_transaction, which is deprecated it becomes clear that starting transaction & ignoring replicate server for next request (session used) while menu link manager rebuilds, looks it needs to research other places

+++ b/core/lib/Drupal/Core/EventSubscriber/MenuRouterRebuildSubscriber.php
@@ -26,6 +27,13 @@ class MenuRouterRebuildSubscriber implements EventSubscriberInterface {
+  private $replicaKillSwitch;

private? I think we don'y need to do that.

@alexpott thanks, good catch!

+++ b/core/includes/database.inc
@@ -1033,18 +1033,13 @@ function db_change_field($table, $field, $field_new, $spec, $keys_new = []) {
+  @trigger_error('db_ignore_replica() is deprecated in Drupal 8.6.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Database\ReplicaKillSwitch::trigger(). See tbd.', E_USER_DEPRECATED);

tbd. Needs a change record.

Sometimes I wonder about whether everything should be a service. I'm not sure. db_ignore_replica() is the only non deprecated function in database.inc - which is kinda amazing and it would be good to move somewhere.

One thing that would be nice is to make the same service listen to the event and set the session. This keeps on the logic in the same place. I think I'd remove the ReplicaDatabaseIgnoreSubscriber and make the new service subscriber to the same event. That way in terms of numbers services this change is a wash.

Added CR draft https://www.drupal.org/node/2997500
Also

1. +++ b/core/includes/database.inc
   @@ -1033,18 +1033,13 @@ function db_change_field($table, $field, $field_new, $spec, $keys_new = []) {
   + * @deprecated as of Drupal 8.6.x, will be removed in Drupal 9.0.0. Use
   ...
    function db_ignore_replica() {
   

   Need to be updated to the 8.7.x

2. +++ b/core/includes/database.inc
   @@ -1033,18 +1033,13 @@ function db_change_field($table, $field, $field_new, $spec, $keys_new = []) {
   +  @trigger_error('db_ignore_replica() is deprecated in Drupal 8.6.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Database\ReplicaKillSwitch::trigger(). See tbd.', E_USER_DEPRECATED);
   

   The same thing here

Reroll + fix of links (CR & naming should point to CR of slavery)
Now it needs a legacy test & update issue summary

It'd be great if someone addressed the last part of #22 ie.

One thing that would be nice is to make the same service listen to the event and set the session. This keeps on the logic in the same place. I think I'd remove the ReplicaDatabaseIgnoreSubscriber and make the new service subscriber to the same event. That way in terms of numbers services this change is a wash.

@alexpott nice idea, guess you mean kind of it

If that fine then it needs legacy test, CR and approval

@andypost yep that looks great. Now we got all the database replica ignoring stuff in the same place. Thanks! Yep a change record would be good. Fortunately event listeners are not API but the deprecation of db_ignore_replica and the new service need one - there's one already so lets make sure it is accurate and there is no harm in mentioning that the old event listener has been removed. +1 for a legacy test

Updated CR https://www.drupal.org/node/2997500

Still, the legacy test needed.

Fixed CR & summary, digging legacy test

Lind of that test covers main functionality

Extended test to test session value

Rerolled

Looks good

Reworded and fixed lines longer 80chars

PS: no idea how to move credits from #2486549: ReplicaDatabaseIgnoreSubscriber mentions non-existent function

Docs update looks fine so back to RTBC.

Unfortunately needs a reroll.

Straight reroll, at least the database.replica service is next to the kill switch in core.services.yml now.

rtbc is back

Committed 788437c and pushed to 8.7.x. Thanks!

diff --git a/core/includes/database.inc b/core/includes/database.inc
index bd19a95c1e..e5927fa65b 100644
--- a/core/includes/database.inc
+++ b/core/includes/database.inc
@@ -11,7 +11,6 @@
 
 use Drupal\Core\Database\Database;
 use Drupal\Core\Database\Query\Condition;
-use Drupal\Core\Site\Settings;
 
 /**
  * @addtogroup database

Fixed unused use on commit.

CR published