[MediaWiki-commits] [Gerrit] RedisBagOStuff: if no alternatives, skip master link status ... - change (mediawiki/core)

2015-07-28 Thread Ori.livneh (Code Review)
Ori.livneh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/227614

Change subject: RedisBagOStuff: if no alternatives, skip master link status 
check
..

RedisBagOStuff: if no alternatives, skip master link status check

If RedisBagOStuff::getConnection() is able to establish a connection, only
check the master link status if automatic failover is enabled and if there are
other viable servers left to consider. If there are no servers left to
consider, or if automatic failover is not configured, just return the
connection handle without subjecting it to further tests.

This will have the side-effect of making RedisBagOStuff compatible with
Nutcracker, which does not implement the INFO command. This is because when
MediaWiki is configured to use Nutcracker, the server pool will consist of a
single server (namely, Nutcracker itself), and thus there will be no other
server to consider, so INFO will never be executed.

Change-Id: I3812ec5a0b22df122bdf44350bc0496574c02ce8
---
M includes/objectcache/RedisBagOStuff.php
1 file changed, 29 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/14/227614/1

diff --git a/includes/objectcache/RedisBagOStuff.php 
b/includes/objectcache/RedisBagOStuff.php
index b8a0dd5..7e506f0 100644
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -372,31 +372,30 @@
}
}
 
-   foreach ( $candidates as $tag ) {
+   while ( ( $tag = array_shift( $candidates ) ) !== false ) {
$server = $this->serverTagMap[$tag];
-
$conn = $this->redisPool->getConnection( $server );
if ( !$conn ) {
continue;
}
 
-   try {
-   $info = $conn->info();
-   // Check if this server has an unreachable 
redis master
-   if ( $info['role'] === 'slave'
-   && $info['master_link_status'] === 
'down'
-   && $this->automaticFailover
-   ) {
-   // If the master cannot be reached, 
fail-over to the next server.
-   // If masters are in data-center A, and 
slaves in data-center B,
-   // this helps avoid the case were 
fail-over happens in A but not
-   // to the corresponding server in B 
(e.g. read/write mismatch).
+   // If automatic failover is enabled, check that the 
server's link
+   // to its master (if any) is up -- but only if there 
are other
+   // viable candidates left to consider.
+   if ( $this->automaticFailover && $candidates ) {
+   try {
+   if ( $this->getMasterLinkStatus( $conn 
) === 'down' ) {
+   // If the master cannot be 
reached, fail-over to the next server.
+   // If masters are in 
data-center A, and slaves in data-center B,
+   // this helps avoid the case 
were fail-over happens in A but not
+   // to the corresponding server 
in B (e.g. read/write mismatch).
+   continue;
+   }
+   } catch ( RedisException $e ) {
+   // Server is not accepting commands
+   $this->handleException( $conn, $e );
continue;
}
-   } catch ( RedisException $e ) {
-   // Server is not accepting commands
-   $this->handleException( $conn, $e );
-   continue;
}
 
return array( $server, $conn );
@@ -408,6 +407,19 @@
}
 
/**
+* Check the master link status of a Redis server that is configured as 
a slave.
+* @param RedisConnRef $conn
+* @return string|null Master link status (either 'up' or 'down'), or 
null
+*  if the server is not a slave.
+*/
+   protected function getMasterLinkStatus( RedisConnRef $conn ) {
+   $info = $conn->info();
+   return isset( $info['master_link_status'] )
+   ? $info['master_link_status']
+   : null;
+   }
+
+   /**
 * Log a 

[MediaWiki-commits] [Gerrit] RedisBagOStuff: if no alternatives, skip master link status ... - change (mediawiki/core)

2015-07-28 Thread Ori.livneh (Code Review)
Ori.livneh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/227617

Change subject: RedisBagOStuff: if no alternatives, skip master link status 
check
..

RedisBagOStuff: if no alternatives, skip master link status check

If RedisBagOStuff::getConnection() is able to establish a connection, only
check the master link status if automatic failover is enabled and if there are
other viable servers left to consider. If there are no servers left to
consider, or if automatic failover is not configured, just return the
connection handle without subjecting it to further tests.

This will have the side-effect of making RedisBagOStuff compatible with
Nutcracker, which does not implement the INFO command. This is because when
MediaWiki is configured to use Nutcracker, the server pool will consist of a
single server (namely, Nutcracker itself), and thus there will be no other
server to consider, so INFO will never be executed.

Change-Id: I3812ec5a0b22df122bdf44350bc0496574c02ce8
---
M includes/objectcache/RedisBagOStuff.php
1 file changed, 29 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/17/227617/1

diff --git a/includes/objectcache/RedisBagOStuff.php 
b/includes/objectcache/RedisBagOStuff.php
index b8a0dd5..7e506f0 100644
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -372,31 +372,30 @@
}
}
 
-   foreach ( $candidates as $tag ) {
+   while ( ( $tag = array_shift( $candidates ) ) !== false ) {
$server = $this->serverTagMap[$tag];
-
$conn = $this->redisPool->getConnection( $server );
if ( !$conn ) {
continue;
}
 
-   try {
-   $info = $conn->info();
-   // Check if this server has an unreachable 
redis master
-   if ( $info['role'] === 'slave'
-   && $info['master_link_status'] === 
'down'
-   && $this->automaticFailover
-   ) {
-   // If the master cannot be reached, 
fail-over to the next server.
-   // If masters are in data-center A, and 
slaves in data-center B,
-   // this helps avoid the case were 
fail-over happens in A but not
-   // to the corresponding server in B 
(e.g. read/write mismatch).
+   // If automatic failover is enabled, check that the 
server's link
+   // to its master (if any) is up -- but only if there 
are other
+   // viable candidates left to consider.
+   if ( $this->automaticFailover && $candidates ) {
+   try {
+   if ( $this->getMasterLinkStatus( $conn 
) === 'down' ) {
+   // If the master cannot be 
reached, fail-over to the next server.
+   // If masters are in 
data-center A, and slaves in data-center B,
+   // this helps avoid the case 
were fail-over happens in A but not
+   // to the corresponding server 
in B (e.g. read/write mismatch).
+   continue;
+   }
+   } catch ( RedisException $e ) {
+   // Server is not accepting commands
+   $this->handleException( $conn, $e );
continue;
}
-   } catch ( RedisException $e ) {
-   // Server is not accepting commands
-   $this->handleException( $conn, $e );
-   continue;
}
 
return array( $server, $conn );
@@ -408,6 +407,19 @@
}
 
/**
+* Check the master link status of a Redis server that is configured as 
a slave.
+* @param RedisConnRef $conn
+* @return string|null Master link status (either 'up' or 'down'), or 
null
+*  if the server is not a slave.
+*/
+   protected function getMasterLinkStatus( RedisConnRef $conn ) {
+   $info = $conn->info();
+   return isset( $info['master_link_status'] )
+   ? $info['master_link_status']
+   : null;
+   }
+
+   /**
 * Log a 

[MediaWiki-commits] [Gerrit] RedisBagOStuff: if no alternatives, skip master link status ... - change (mediawiki/core)

2015-07-28 Thread Ori.livneh (Code Review)
Ori.livneh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/227618

Change subject: RedisBagOStuff: if no alternatives, skip master link status 
check
..

RedisBagOStuff: if no alternatives, skip master link status check

If RedisBagOStuff::getConnection() is able to establish a connection, only
check the master link status if automatic failover is enabled and if there are
other viable servers left to consider. If there are no servers left to
consider, or if automatic failover is not configured, just return the
connection handle without subjecting it to further tests.

This will have the side-effect of making RedisBagOStuff compatible with
Nutcracker, which does not implement the INFO command. This is because when
MediaWiki is configured to use Nutcracker, the server pool will consist of a
single server (namely, Nutcracker itself), and thus there will be no other
server to consider, so INFO will never be executed.

Change-Id: I3812ec5a0b22df122bdf44350bc0496574c02ce8
---
M includes/objectcache/RedisBagOStuff.php
1 file changed, 29 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/18/227618/1

diff --git a/includes/objectcache/RedisBagOStuff.php 
b/includes/objectcache/RedisBagOStuff.php
index b8a0dd5..7e506f0 100644
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -372,31 +372,30 @@
}
}
 
-   foreach ( $candidates as $tag ) {
+   while ( ( $tag = array_shift( $candidates ) ) !== false ) {
$server = $this->serverTagMap[$tag];
-
$conn = $this->redisPool->getConnection( $server );
if ( !$conn ) {
continue;
}
 
-   try {
-   $info = $conn->info();
-   // Check if this server has an unreachable 
redis master
-   if ( $info['role'] === 'slave'
-   && $info['master_link_status'] === 
'down'
-   && $this->automaticFailover
-   ) {
-   // If the master cannot be reached, 
fail-over to the next server.
-   // If masters are in data-center A, and 
slaves in data-center B,
-   // this helps avoid the case were 
fail-over happens in A but not
-   // to the corresponding server in B 
(e.g. read/write mismatch).
+   // If automatic failover is enabled, check that the 
server's link
+   // to its master (if any) is up -- but only if there 
are other
+   // viable candidates left to consider.
+   if ( $this->automaticFailover && $candidates ) {
+   try {
+   if ( $this->getMasterLinkStatus( $conn 
) === 'down' ) {
+   // If the master cannot be 
reached, fail-over to the next server.
+   // If masters are in 
data-center A, and slaves in data-center B,
+   // this helps avoid the case 
were fail-over happens in A but not
+   // to the corresponding server 
in B (e.g. read/write mismatch).
+   continue;
+   }
+   } catch ( RedisException $e ) {
+   // Server is not accepting commands
+   $this->handleException( $conn, $e );
continue;
}
-   } catch ( RedisException $e ) {
-   // Server is not accepting commands
-   $this->handleException( $conn, $e );
-   continue;
}
 
return array( $server, $conn );
@@ -408,6 +407,19 @@
}
 
/**
+* Check the master link status of a Redis server that is configured as 
a slave.
+* @param RedisConnRef $conn
+* @return string|null Master link status (either 'up' or 'down'), or 
null
+*  if the server is not a slave.
+*/
+   protected function getMasterLinkStatus( RedisConnRef $conn ) {
+   $info = $conn->info();
+   return isset( $info['master_link_status'] )
+   ? $info['master_link_status']
+   : null;
+   }
+
+   /**
 * Log a 

[MediaWiki-commits] [Gerrit] RedisBagOStuff: if no alternatives, skip master link status ... - change (mediawiki/core)

2015-07-28 Thread Ori.livneh (Code Review)
Ori.livneh has submitted this change and it was merged.

Change subject: RedisBagOStuff: if no alternatives, skip master link status 
check
..


RedisBagOStuff: if no alternatives, skip master link status check

If RedisBagOStuff::getConnection() is able to establish a connection, only
check the master link status if automatic failover is enabled and if there are
other viable servers left to consider. If there are no servers left to
consider, or if automatic failover is not configured, just return the
connection handle without subjecting it to further tests.

This will have the side-effect of making RedisBagOStuff compatible with
Nutcracker, which does not implement the INFO command. This is because when
MediaWiki is configured to use Nutcracker, the server pool will consist of a
single server (namely, Nutcracker itself), and thus there will be no other
server to consider, so INFO will never be executed.

Change-Id: I3812ec5a0b22df122bdf44350bc0496574c02ce8
---
M includes/objectcache/RedisBagOStuff.php
1 file changed, 29 insertions(+), 17 deletions(-)

Approvals:
  Ori.livneh: Verified; Looks good to me, approved



diff --git a/includes/objectcache/RedisBagOStuff.php 
b/includes/objectcache/RedisBagOStuff.php
index b8a0dd5..7e506f0 100644
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -372,31 +372,30 @@
}
}
 
-   foreach ( $candidates as $tag ) {
+   while ( ( $tag = array_shift( $candidates ) ) !== false ) {
$server = $this->serverTagMap[$tag];
-
$conn = $this->redisPool->getConnection( $server );
if ( !$conn ) {
continue;
}
 
-   try {
-   $info = $conn->info();
-   // Check if this server has an unreachable 
redis master
-   if ( $info['role'] === 'slave'
-   && $info['master_link_status'] === 
'down'
-   && $this->automaticFailover
-   ) {
-   // If the master cannot be reached, 
fail-over to the next server.
-   // If masters are in data-center A, and 
slaves in data-center B,
-   // this helps avoid the case were 
fail-over happens in A but not
-   // to the corresponding server in B 
(e.g. read/write mismatch).
+   // If automatic failover is enabled, check that the 
server's link
+   // to its master (if any) is up -- but only if there 
are other
+   // viable candidates left to consider.
+   if ( $this->automaticFailover && $candidates ) {
+   try {
+   if ( $this->getMasterLinkStatus( $conn 
) === 'down' ) {
+   // If the master cannot be 
reached, fail-over to the next server.
+   // If masters are in 
data-center A, and slaves in data-center B,
+   // this helps avoid the case 
were fail-over happens in A but not
+   // to the corresponding server 
in B (e.g. read/write mismatch).
+   continue;
+   }
+   } catch ( RedisException $e ) {
+   // Server is not accepting commands
+   $this->handleException( $conn, $e );
continue;
}
-   } catch ( RedisException $e ) {
-   // Server is not accepting commands
-   $this->handleException( $conn, $e );
-   continue;
}
 
return array( $server, $conn );
@@ -408,6 +407,19 @@
}
 
/**
+* Check the master link status of a Redis server that is configured as 
a slave.
+* @param RedisConnRef $conn
+* @return string|null Master link status (either 'up' or 'down'), or 
null
+*  if the server is not a slave.
+*/
+   protected function getMasterLinkStatus( RedisConnRef $conn ) {
+   $info = $conn->info();
+   return isset( $info['master_link_status'] )
+   ? $info['master_link_status']
+   : null;
+   }
+
+   /**
 * Log a fatal error
 * @param string $msg
 */


[MediaWiki-commits] [Gerrit] RedisBagOStuff: if no alternatives, skip master link status ... - change (mediawiki/core)

2015-07-28 Thread Ori.livneh (Code Review)
Ori.livneh has submitted this change and it was merged.

Change subject: RedisBagOStuff: if no alternatives, skip master link status 
check
..


RedisBagOStuff: if no alternatives, skip master link status check

If RedisBagOStuff::getConnection() is able to establish a connection, only
check the master link status if automatic failover is enabled and if there are
other viable servers left to consider. If there are no servers left to
consider, or if automatic failover is not configured, just return the
connection handle without subjecting it to further tests.

This will have the side-effect of making RedisBagOStuff compatible with
Nutcracker, which does not implement the INFO command. This is because when
MediaWiki is configured to use Nutcracker, the server pool will consist of a
single server (namely, Nutcracker itself), and thus there will be no other
server to consider, so INFO will never be executed.

Change-Id: I3812ec5a0b22df122bdf44350bc0496574c02ce8
---
M includes/objectcache/RedisBagOStuff.php
1 file changed, 29 insertions(+), 17 deletions(-)

Approvals:
  Ori.livneh: Verified; Looks good to me, approved



diff --git a/includes/objectcache/RedisBagOStuff.php 
b/includes/objectcache/RedisBagOStuff.php
index b8a0dd5..7e506f0 100644
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -372,31 +372,30 @@
}
}
 
-   foreach ( $candidates as $tag ) {
+   while ( ( $tag = array_shift( $candidates ) ) !== false ) {
$server = $this->serverTagMap[$tag];
-
$conn = $this->redisPool->getConnection( $server );
if ( !$conn ) {
continue;
}
 
-   try {
-   $info = $conn->info();
-   // Check if this server has an unreachable 
redis master
-   if ( $info['role'] === 'slave'
-   && $info['master_link_status'] === 
'down'
-   && $this->automaticFailover
-   ) {
-   // If the master cannot be reached, 
fail-over to the next server.
-   // If masters are in data-center A, and 
slaves in data-center B,
-   // this helps avoid the case were 
fail-over happens in A but not
-   // to the corresponding server in B 
(e.g. read/write mismatch).
+   // If automatic failover is enabled, check that the 
server's link
+   // to its master (if any) is up -- but only if there 
are other
+   // viable candidates left to consider.
+   if ( $this->automaticFailover && $candidates ) {
+   try {
+   if ( $this->getMasterLinkStatus( $conn 
) === 'down' ) {
+   // If the master cannot be 
reached, fail-over to the next server.
+   // If masters are in 
data-center A, and slaves in data-center B,
+   // this helps avoid the case 
were fail-over happens in A but not
+   // to the corresponding server 
in B (e.g. read/write mismatch).
+   continue;
+   }
+   } catch ( RedisException $e ) {
+   // Server is not accepting commands
+   $this->handleException( $conn, $e );
continue;
}
-   } catch ( RedisException $e ) {
-   // Server is not accepting commands
-   $this->handleException( $conn, $e );
-   continue;
}
 
return array( $server, $conn );
@@ -408,6 +407,19 @@
}
 
/**
+* Check the master link status of a Redis server that is configured as 
a slave.
+* @param RedisConnRef $conn
+* @return string|null Master link status (either 'up' or 'down'), or 
null
+*  if the server is not a slave.
+*/
+   protected function getMasterLinkStatus( RedisConnRef $conn ) {
+   $info = $conn->info();
+   return isset( $info['master_link_status'] )
+   ? $info['master_link_status']
+   : null;
+   }
+
+   /**
 * Log a fatal error
 * @param string $msg
 */


[MediaWiki-commits] [Gerrit] RedisBagOStuff: if no alternatives, skip master link status ... - change (mediawiki/core)

2015-07-28 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: RedisBagOStuff: if no alternatives, skip master link status 
check
..


RedisBagOStuff: if no alternatives, skip master link status check

If RedisBagOStuff::getConnection() is able to establish a connection, only
check the master link status if automatic failover is enabled and if there are
other viable servers left to consider. If there are no servers left to
consider, or if automatic failover is not configured, just return the
connection handle without subjecting it to further tests.

This will have the side-effect of making RedisBagOStuff compatible with
Nutcracker, which does not implement the INFO command. This is because when
MediaWiki is configured to use Nutcracker, the server pool will consist of a
single server (namely, Nutcracker itself), and thus there will be no other
server to consider, so INFO will never be executed.

Change-Id: I3812ec5a0b22df122bdf44350bc0496574c02ce8
---
M includes/objectcache/RedisBagOStuff.php
1 file changed, 29 insertions(+), 17 deletions(-)

Approvals:
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/objectcache/RedisBagOStuff.php 
b/includes/objectcache/RedisBagOStuff.php
index b8a0dd5..7e506f0 100644
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -372,31 +372,30 @@
}
}
 
-   foreach ( $candidates as $tag ) {
+   while ( ( $tag = array_shift( $candidates ) ) !== false ) {
$server = $this->serverTagMap[$tag];
-
$conn = $this->redisPool->getConnection( $server );
if ( !$conn ) {
continue;
}
 
-   try {
-   $info = $conn->info();
-   // Check if this server has an unreachable 
redis master
-   if ( $info['role'] === 'slave'
-   && $info['master_link_status'] === 
'down'
-   && $this->automaticFailover
-   ) {
-   // If the master cannot be reached, 
fail-over to the next server.
-   // If masters are in data-center A, and 
slaves in data-center B,
-   // this helps avoid the case were 
fail-over happens in A but not
-   // to the corresponding server in B 
(e.g. read/write mismatch).
+   // If automatic failover is enabled, check that the 
server's link
+   // to its master (if any) is up -- but only if there 
are other
+   // viable candidates left to consider.
+   if ( $this->automaticFailover && $candidates ) {
+   try {
+   if ( $this->getMasterLinkStatus( $conn 
) === 'down' ) {
+   // If the master cannot be 
reached, fail-over to the next server.
+   // If masters are in 
data-center A, and slaves in data-center B,
+   // this helps avoid the case 
were fail-over happens in A but not
+   // to the corresponding server 
in B (e.g. read/write mismatch).
+   continue;
+   }
+   } catch ( RedisException $e ) {
+   // Server is not accepting commands
+   $this->handleException( $conn, $e );
continue;
}
-   } catch ( RedisException $e ) {
-   // Server is not accepting commands
-   $this->handleException( $conn, $e );
-   continue;
}
 
return array( $server, $conn );
@@ -408,6 +407,19 @@
}
 
/**
+* Check the master link status of a Redis server that is configured as 
a slave.
+* @param RedisConnRef $conn
+* @return string|null Master link status (either 'up' or 'down'), or 
null
+*  if the server is not a slave.
+*/
+   protected function getMasterLinkStatus( RedisConnRef $conn ) {
+   $info = $conn->info();
+   return isset( $info['master_link_status'] )
+   ? $info['master_link_status']
+   : null;
+   }
+
+   /**
 * Log a fatal error
 * @param string $