Skip to content

Commit

Permalink
Improve control connection retries. Code refactoring. Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ashetkar committed May 19, 2024
1 parent 1b4413e commit 4ddaabc
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 74 deletions.
4 changes: 2 additions & 2 deletions examples/uniform_load_balance_run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ $INSTALL_DIR/bin/yb-ctl destroy > yb-ctl.log 2>&1

echo "Creating a 3-node, RF-3 cluster (live nodes: 1,2,3)"
$INSTALL_DIR/bin/yb-ctl create --rf 3 >> yb-ctl.log 2>&1
SLEEP 5
SLEEP 5 # Allow some time for the cluster to be ready

#deleting the checker file if exists
verbosePrint $VERBOSE "Deleting all the temporary checker files if exists"
Expand All @@ -106,7 +106,7 @@ interact $INTERACTIVE

echoSleep "Adding Node-4 to the cluster (live nodes: 1,2,3,4)"
$INSTALL_DIR/bin/yb-ctl add_node >> yb-ctl.log 2>&1
SLEEP 5
SLEEP 5 # Allow some time for the node to be ready

touch .jdbc_example_app_checker #resuming the java app

Expand Down
64 changes: 44 additions & 20 deletions pgjdbc/src/main/java/com/yugabyte/ysql/LoadBalanceManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ public static void printHostToConnectionMap() {
}
}

/**
* FOR TEST PURPOSE ONLY
*/
public static synchronized void clear() {
LOGGER.warning("Clearing LoadBalanceManager state for testing purposes.");
clusterInfoMap.clear();
controlConnection = null;
lastRefreshTime = 0;
forceRefreshOnce = false;
useHostColumn = null;
}

public static long getLastRefreshTime() {
return lastRefreshTime;
}
Expand Down Expand Up @@ -231,7 +243,7 @@ public static Connection getConnection(String url, Properties properties, String
properties);
// Cleanup extra properties used for load balancing?
if (lbProperties.hasLoadBalance()) {
Connection conn = getConnection(lbProperties, user, database);
Connection conn = getConnection(lbProperties, properties, user, database);
if (conn != null) {
return conn;
}
Expand All @@ -240,14 +252,13 @@ public static Connection getConnection(String url, Properties properties, String
return null;
}

public static Connection getConnection(LoadBalanceProperties loadBalanceProperties, String user,
String dbName) {
public static Connection getConnection(LoadBalanceProperties loadBalanceProperties,
Properties props, String user, String dbName) {
LoadBalancer lb = loadBalanceProperties.getAppropriateLoadBalancer();
Properties props = loadBalanceProperties.getOriginalProperties();
String url = loadBalanceProperties.getStrippedURL();

if (!checkAndRefresh(loadBalanceProperties, lb, user, dbName)) {
LOGGER.fine("checkAndRefresh() returns false");
LOGGER.fine("Attempt to refresh info from yb_servers() failed");
return null;
}

Expand Down Expand Up @@ -292,6 +303,14 @@ public static Connection getConnection(LoadBalanceProperties loadBalanceProperti
return null;
}

/**
*
* @param loadBalanceProperties
* @param lb LoadBalancer instance
* @param user
* @param dbName
* @return true if the refresh was not required or if it was successful.
*/
private static synchronized boolean checkAndRefresh(LoadBalanceProperties loadBalanceProperties,
LoadBalancer lb, String user, String dbName) {
if (needsRefresh(lb.getRefreshListSeconds())) {
Expand All @@ -317,27 +336,32 @@ private static synchronized boolean checkAndRefresh(LoadBalanceProperties loadBa
} catch (SQLException ex) {
if (refreshFailed) {
LOGGER.fine("Exception while refreshing: " + ex + ", " + ex.getSQLState());
String failed = ((PgConnection) controlConnection).getQueryExecutor().getHostSpec().getHost();
markAsFailed(failed);
if (hspec.length > 1) {
HostSpec[] newHspec = new HostSpec[hspec.length - 1];
for (int i = 0, j = 0; i < hspec.length; i++) {
if (!failed.equalsIgnoreCase(hspec[i].getHost())) {
newHspec[j] = hspec[i];
j++;
}
}
hspec = newHspec;
}
} else {
String msg = hspec.length > 1 ? " and others" : "";
LOGGER.fine("Exception while creating control connection to "
+ hspec[0].getHost() + ": " + ex + ", " + ex.getSQLState());
}
if (PSQLState.UNDEFINED_FUNCTION.getState().equals(ex.getSQLState())) {
LOGGER.warning("Received error UNDEFINED_FUNCTION (42883)");
return false;
}
if (PSQLState.CONNECTION_UNABLE_TO_CONNECT.getState().equals(ex.getSQLState())) {
LOGGER.warning("Received error CONNECTION_UNABLE_TO_CONNECT for " + hspec[0].getHost());
for (HostSpec h : hspec) {
markAsFailed(h.getHost());
}
}
// Remove hspec only if exception not thrown from refresh() above
// Since it could be a stale connection
if (!refreshFailed) {
+ hspec[0].getHost() + msg + ": " + ex + ", " + ex.getSQLState());
for (HostSpec h : hspec) {
hosts.remove(h.getHost());
}
}
if (PSQLState.UNDEFINED_FUNCTION.getState().equals(ex.getSQLState())) {
LOGGER.warning("Received UNDEFINED_FUNCTION for yb_servers()" +
" (SQLState=42883). You may be using an older version of" +
" YugabyteDB, consider upgrading it.");
return false;
}
// Retry until servers are available
if (hosts.isEmpty()) {
LOGGER.fine("Failed to establish control connection to available servers");
Expand Down
87 changes: 35 additions & 52 deletions pgjdbc/src/main/java/com/yugabyte/ysql/LoadBalanceProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,21 +130,9 @@ public String processURLAndProperties() {
"Ignoring it.");
continue;
}
try {
refreshInterval = Integer.parseInt(lbParts[1]);
if (refreshInterval < 0 || refreshInterval > MAX_REFRESH_INTERVAL) {
LOGGER.warning("Provided yb-servers-refresh-interval ("
+ refreshInterval + ") is outside the permissible range,"
+ " setting it to 300 seconds");
refreshInterval = DEFAULT_REFRESH_INTERVAL;
} else {
refreshIntervalSpecified = true;
}
} catch (NumberFormatException nfe) {
LOGGER.warning("Provided yb-servers-refresh-interval ("
+ refreshInterval + ") is invalid, setting it to 300 seconds");
refreshInterval = DEFAULT_REFRESH_INTERVAL;
}
refreshIntervalSpecified = true;
refreshInterval = parseAndGetValue(lbParts[1],
DEFAULT_REFRESH_INTERVAL, MAX_REFRESH_INTERVAL);
} else if (part.startsWith(explicitFallbackOnlyKey)) {
String[] lbParts = part.split(EQUALS);
if (lbParts.length != 2) {
Expand All @@ -162,16 +150,9 @@ public String processURLAndProperties() {
"Ignoring it.");
continue;
}
try {
failedHostReconnectDelaySecs = Integer.parseInt(lbParts[1]);
if (failedHostReconnectDelaySecs < 0 || failedHostReconnectDelaySecs > MAX_FAILED_HOST_RECONNECT_DELAY_SECS) {
failedHostReconnectDelaySecs = DEFAULT_FAILED_HOST_TTL_SECONDS;
} else {
failedHostReconnectDelaySpecified = true;
}
} catch (NumberFormatException nfe) {
failedHostReconnectDelaySecs = DEFAULT_FAILED_HOST_TTL_SECONDS;
}
failedHostReconnectDelaySpecified = true;
failedHostReconnectDelaySecs = parseAndGetValue(lbParts[1],
DEFAULT_FAILED_HOST_TTL_SECONDS, MAX_FAILED_HOST_RECONNECT_DELAY_SECS);
} else {
if (sb.toString().contains("?")) {
sb.append(PROPERTY_SEP);
Expand All @@ -195,22 +176,8 @@ public String processURLAndProperties() {
placements = propValue;
}
if (originalProperties.containsKey(REFRESH_INTERVAL_KEY)) {
String propValue = originalProperties.getProperty(REFRESH_INTERVAL_KEY);
try {
refreshInterval = Integer.parseInt(propValue);
if (refreshInterval < 0 || refreshInterval > MAX_REFRESH_INTERVAL) {
LOGGER.warning("Provided yb-servers-refresh-interval ("
+ refreshInterval + ") is outside the permissible range,"
+ " setting it to 300 seconds");
refreshInterval = DEFAULT_REFRESH_INTERVAL;
} else {
refreshIntervalSpecified = true;
}
} catch (NumberFormatException nfe) {
LOGGER.warning("Provided yb-servers-refresh-interval ("
+ refreshInterval + ") is invalid, setting it to 300 seconds");
refreshInterval = DEFAULT_REFRESH_INTERVAL;
}
refreshInterval = parseAndGetValue(originalProperties.getProperty(REFRESH_INTERVAL_KEY),
DEFAULT_REFRESH_INTERVAL, MAX_REFRESH_INTERVAL);
}
if (originalProperties.containsKey(EXPLICIT_FALLBACK_ONLY_KEY)) {
String propValue = originalProperties.getProperty(EXPLICIT_FALLBACK_ONLY_KEY);
Expand All @@ -219,22 +186,29 @@ public String processURLAndProperties() {
}
}
if (originalProperties.containsKey(FAILED_HOST_RECONNECT_DELAY_SECS_KEY)) {
String propValue = originalProperties.getProperty(FAILED_HOST_RECONNECT_DELAY_SECS_KEY);
try {
failedHostReconnectDelaySecs = Integer.parseInt(propValue);
if (failedHostReconnectDelaySecs < 0 || failedHostReconnectDelaySecs > MAX_FAILED_HOST_RECONNECT_DELAY_SECS) {
failedHostReconnectDelaySecs = DEFAULT_FAILED_HOST_TTL_SECONDS;
} else {
failedHostReconnectDelaySpecified = true;
}
} catch (NumberFormatException nfe) {
failedHostReconnectDelaySecs = DEFAULT_FAILED_HOST_TTL_SECONDS;
}
failedHostReconnectDelaySecs =
parseAndGetValue(originalProperties.getProperty(FAILED_HOST_RECONNECT_DELAY_SECS_KEY),
DEFAULT_FAILED_HOST_TTL_SECONDS, MAX_FAILED_HOST_RECONNECT_DELAY_SECS);
}
}
return sb.toString();
}

private int parseAndGetValue(String propValue, int defaultValue, int maxValue) {
try {
int value = Integer.parseInt(propValue);
if (value < 0 || value > maxValue) {
LOGGER.warning("Provided value (" + value + ") is outside the permissible range,"
+ " using the default value instead");
return defaultValue;
}
return value;
} catch (NumberFormatException nfe) {
LOGGER.warning("Provided value (" + propValue + ") is invalid, using the default value instead");
return defaultValue;
}
}

public String getOriginalURL() {
return originalUrl;
}
Expand Down Expand Up @@ -306,6 +280,15 @@ public LoadBalancerKey(String url, Properties properties) {
this.properties = properties;
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((url == null) ? 0 : url.hashCode());
result = prime * result + ((properties == null) ? 0 : properties.hashCode());
return result;
}

public boolean equals(Object other) {
return other instanceof LoadBalancerKey &&
url != null && url.equals(((LoadBalancerKey) other).url) &&
Expand Down
4 changes: 4 additions & 0 deletions pgjdbc/src/test/java/com/yugabyte/FallbackOptionsLBTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ private static void checkBasicBehavior() throws SQLException {
createConnectionsAndVerify(baseUrl, "aws.us-west.us-west-2a:1,aws.us-west.us-west-2b:2,aws" + ".us-west.us-west-2c:", null);
} finally {
CONNECTION_MANAGER_MAP.clear();
LoadBalanceManager.clear();
executeCmd(path + "/bin/yb-ctl destroy", "Stop YugabyteDB cluster", 10);
}
}
Expand Down Expand Up @@ -107,6 +108,7 @@ private static void checkNodeDownBehavior() throws SQLException {
"aws.us-west.us-west-1a", expectedInput(-1, -1, -1, 12+1, 0, 0));
} finally {
CONNECTION_MANAGER_MAP.clear();
LoadBalanceManager.clear();
executeCmd(path + "/bin/yb-ctl destroy", "Stop YugabyteDB cluster", 10);
}
}
Expand All @@ -122,6 +124,7 @@ private static void checkNodeDownBehaviorMultiFallback() throws SQLException {
".7:5433/yugabyte?load-balance=true&yb-servers-refresh-interval=0&topology-keys=";

try {
// +1 is for control connection
controlHost = "127.0.0.1";
String tk = "aws.us-west.*:1,aws.us-east.*:2,aws.eu-west.*:3,aws.eu-north.*:4";
createConnectionsAndVerify(url, tk, expectedInput(4+1, 4, 4, 0, 0, 0, 0, 0, 0));
Expand Down Expand Up @@ -160,6 +163,7 @@ private static void checkNodeDownBehaviorMultiFallback() throws SQLException {

} finally {
CONNECTION_MANAGER_MAP.clear();
LoadBalanceManager.clear();
executeCmd(path + "/bin/yb-ctl destroy", "Stop YugabyteDB cluster", 10);
}
}
Expand Down

0 comments on commit 4ddaabc

Please sign in to comment.