Skip to content

Commit

Permalink
[Platform][CLOUDGA-4886] fix 500 errors when requesting slow queries …
Browse files Browse the repository at this point in the history
…during a rolling restart

Summary:
If a JDBC connection to a DB cannot be made, slow query executor will fail with an NPE due to the
resposne JSON not having a type property set. This results in 500 error reported to the client with
a misleading message about the universe not having a private IP or DNS.

Test Plan: Create a universe, request slow queries during a restart.

Reviewers: sb-yb, andrew, daniel

Reviewed By: daniel

Subscribers: jenkins-bot, yugaware

Differential Revision: https://phabricator.dev.yugabyte.com/D15994
  • Loading branch information
artem-mindrov committed Mar 25, 2022
1 parent 97696cf commit 0295e25
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,7 @@ public Result getSlowQueries(UUID customerUUID, UUID universeUUID) {
Optional<String> optPassword = request().getHeaders().get(YSQL_PASSWORD_HEADER);
JsonNode resultNode =
universeInfoHandler.getSlowQueries(
universe,
optUsername.orElse(null),
optPassword.isPresent() ? Util.decodeBase64(optPassword.get()) : null);
universe, optUsername.orElse(null), optPassword.map(Util::decodeBase64).orElse(null));
return Results.ok(resultNode);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,9 @@ public JsonNode getLiveQuery(Universe universe) {
JsonNode resultNode;
try {
resultNode = queryHelper.liveQueries(universe);
} catch (NullPointerException e) {
log.error("Universe does not have a private IP or DNS", e);
throw new PlatformServiceException(
INTERNAL_SERVER_ERROR, "Universe failed to fetch live queries");
} catch (Throwable t) {
log.error("Error retrieving queries for universe", t);
throw new PlatformServiceException(INTERNAL_SERVER_ERROR, t.getMessage());
} catch (Exception e) {
log.error("Error retrieving queries for universe", e);
throw new PlatformServiceException(INTERNAL_SERVER_ERROR, e.getMessage());
}
return resultNode;
}
Expand All @@ -168,28 +164,19 @@ public JsonNode getSlowQueries(Universe universe, String username, String passwo
} catch (IllegalArgumentException e) {
log.error(e.getMessage(), e);
throw new PlatformServiceException(BAD_REQUEST, e.getMessage());
} catch (NullPointerException e) {
log.error("Universe does not have a private IP or DNS", e);
throw new PlatformServiceException(
INTERNAL_SERVER_ERROR, "Universe failed to fetch slow queries");
} catch (Throwable t) {
log.error("Error retrieving queries for universe", t);
throw new PlatformServiceException(INTERNAL_SERVER_ERROR, t.getMessage());
} catch (Exception e) {
log.error("Error retrieving queries for universe", e);
throw new PlatformServiceException(INTERNAL_SERVER_ERROR, e.getMessage());
}
return resultNode;
}

public JsonNode resetSlowQueries(Universe universe) {
try {
return queryHelper.resetQueries(universe);
} catch (NullPointerException e) {
// TODO: Investigate why catch NPE??
throw new PlatformServiceException(
INTERNAL_SERVER_ERROR, "Failed reach node, invalid IP or DNS.");
} catch (Throwable t) {
// TODO: Investigate why catch Throwable??
log.error("Error resetting slow queries for universe", t);
throw new PlatformServiceException(INTERNAL_SERVER_ERROR, t.getMessage());
} catch (Exception e) {
log.error("Error resetting slow queries for universe", e);
throw new PlatformServiceException(INTERNAL_SERVER_ERROR, e.getMessage());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,22 @@
import com.yugabyte.yw.forms.LiveQueriesParams;
import java.util.UUID;
import java.util.concurrent.Callable;

import lombok.extern.slf4j.Slf4j;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import play.api.Play;
import play.libs.Json;

@Slf4j
public class LiveQueryExecutor implements Callable<JsonNode> {
public static final Logger LOG = LoggerFactory.getLogger(LiveQueryExecutor.class);

private final ApiHelper apiHelper;
// hostname can be either IP address or DNS
private String hostName;
private String nodeName;
private int port;
private QueryHelper.QueryApi apiType;
private final String hostName;
private final String nodeName;
private final int port;
private final QueryHelper.QueryApi apiType;

public LiveQueryExecutor(String nodeName, String hostName, int port, QueryHelper.QueryApi api) {
this.nodeName = nodeName;
Expand All @@ -45,7 +47,7 @@ public JsonNode call() throws Exception {
return processYCQLRowData(response);
}
} catch (Exception e) {
LOG.error("Exception while fetching url: {}; message: {}", url, e.getStackTrace());
log.error(String.format("Exception while fetching url: %s", url), e);
ObjectNode errorJson = Json.newObject();
errorJson.put("error", e.getMessage());
errorJson.put("type", apiType == QueryHelper.QueryApi.YSQL ? "ysql" : "ycql");
Expand Down Expand Up @@ -80,16 +82,18 @@ private JsonNode processYSQLRowData(JsonNode response) {
rowData.put("appName", params.application_name);
rowData.put("clientHost", params.host);
rowData.put("clientPort", params.port);

ArrayNode ysqlArray;
if (!responseJson.has("ysql")) {
ArrayNode ysqlArray = responseJson.putArray("ysql");
ysqlArray.add(rowData);
ysqlArray = responseJson.putArray("ysql");
} else {
ArrayNode ysqlArray = (ArrayNode) responseJson.get("ysql");
ysqlArray.add(rowData);
ysqlArray = (ArrayNode) responseJson.get("ysql");
}
} catch (JsonProcessingException exception) {

ysqlArray.add(rowData);
} catch (JsonProcessingException e) {
// Try to process all connections even if there is an exception
LOG.error("Unable to process JSON from YSQL query. {}", objNode.asText());
log.error(e.getMessage(), e);
}
}
}
Expand Down Expand Up @@ -131,17 +135,18 @@ private JsonNode processYCQLRowData(JsonNode response) {
rowData.put("clientHost", splitIp[0]);
rowData.put("clientPort", splitIp[1]);

ArrayNode ycqlArray;
if (!responseJson.has("ycql")) {
ArrayNode ycqlArray = responseJson.putArray("ycql");
ycqlArray.add(rowData);
ycqlArray = responseJson.putArray("ycql");
} else {
ArrayNode ycqlArray = (ArrayNode) responseJson.get("ycql");
ycqlArray.add(rowData);
ycqlArray = (ArrayNode) responseJson.get("ycql");
}

ycqlArray.add(rowData);
}
}
} catch (JsonProcessingException exception) {
LOG.error("Unable to process JSON from YCQL query connection", objNode.asText());
} catch (JsonProcessingException e) {
log.error(e.getMessage(), e);
}
}
}
Expand Down
59 changes: 39 additions & 20 deletions managed/src/main/java/com/yugabyte/yw/queries/QueryHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.yugabyte.yw.common.YsqlQueryExecutor;
import com.yugabyte.yw.forms.RunQueryFormData;
import com.yugabyte.yw.models.Universe;
import com.yugabyte.yw.models.helpers.CloudSpecificInfo;
import com.yugabyte.yw.models.helpers.NodeDetails;
import java.util.Arrays;
import java.util.HashMap;
Expand All @@ -20,13 +21,13 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import lombok.extern.slf4j.Slf4j;
import play.libs.Json;

@Slf4j
@Singleton
public class QueryHelper {
public static final Logger LOG = LoggerFactory.getLogger(QueryHelper.class);
public static final Integer QUERY_EXECUTOR_THREAD_POOL = 5;

private static final String SLOW_QUERY_STATS_SQL =
Expand Down Expand Up @@ -65,19 +66,30 @@ public JsonNode query(
throws IllegalArgumentException {
ExecutorService threadPool = Executors.newFixedThreadPool(QUERY_EXECUTOR_THREAD_POOL);
Set<Future<JsonNode>> futures = new HashSet<>();
int ysqlErrorCount = 0;
int ycqlErrorCount = 0;
ObjectNode responseJson = Json.newObject();
ObjectNode ysqlJson = Json.newObject();
ysqlJson.put("errorCount", 0);
ysqlJson.putArray("queries");
ObjectNode ycqlJson = Json.newObject();
ycqlJson.put("errorCount", 0);
ycqlJson.putArray("queries");
for (NodeDetails node : universe.getNodes()) {
if (node.isActive() && node.isTserver) {
String ip =
node.cloudInfo.private_ip == null
? node.cloudInfo.private_dns
: node.cloudInfo.private_ip;
String ip = null;
CloudSpecificInfo cloudInfo = node.cloudInfo;

if (cloudInfo != null) {
ip =
node.cloudInfo.private_ip == null
? node.cloudInfo.private_dns
: node.cloudInfo.private_ip;
}

if (ip == null) {
log.error("Node {} does not have a private IP or DNS name, skipping", node.nodeName);
continue;
}

Callable<JsonNode> callable;

if (fetchSlowQueries) {
Expand All @@ -101,6 +113,13 @@ public JsonNode query(
}
}

if (futures.isEmpty()) {
throw new IllegalStateException(
"None of the nodes are accessible by either private IP or DNS");
}

threadPool.shutdown();

try {
Map<String, JsonNode> queryMap = new HashMap<>();
for (Future<JsonNode> future : futures) {
Expand All @@ -112,11 +131,13 @@ public JsonNode query(
if (errorMessage.startsWith("\"FATAL: password authentication failed")) {
throw new IllegalArgumentException("Incorrect Username or Password");
}
String type = response.get("type").asText();
if ("ysql".equals(type)) {
ysqlJson.put("errorCount", ysqlJson.get("errorCount").asInt() + 1);
} else if ("ycql".equals(type)) {
ycqlJson.put("errorCount", ycqlJson.get("errorCount").asInt() + 1);
if (response.has("type")) {
String type = response.get("type").asText();
if ("ysql".equals(type)) {
ysqlErrorCount++;
} else if ("ycql".equals(type)) {
ycqlErrorCount++;
}
}
} else {
if (fetchSlowQueries) {
Expand Down Expand Up @@ -193,18 +214,16 @@ public JsonNode query(
}
}
} catch (InterruptedException e) {
LOG.error("Error fetching live query data", e);
log.error("Error fetching live query data", e);
} catch (ExecutionException e) {
LOG.error("Error fetching live query data", e);
e.printStackTrace();
} finally {
threadPool.shutdown();
log.error("Error fetching live query data", e.getCause());
}

ysqlJson.put("errorCount", ysqlErrorCount);
ycqlJson.put("errorCount", ycqlErrorCount);
responseJson.set("ysql", ysqlJson);
responseJson.set("ycql", ycqlJson);

threadPool.shutdown();
return responseJson;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.yugabyte.yw.common.ApiHelper;
import com.yugabyte.yw.forms.UniverseDefinitionTaskParams;
import com.yugabyte.yw.models.Universe;
import java.sql.Connection;
Expand All @@ -21,25 +20,20 @@
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.Callable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import play.api.Play;

import play.libs.Json;
import lombok.extern.slf4j.Slf4j;

@Slf4j
public class SlowQueryExecutor implements Callable<JsonNode> {
public static final Logger LOG = LoggerFactory.getLogger(LiveQueryExecutor.class);

private final ApiHelper apiHelper;
// hostname can be either IP address or DNS
private String hostName;
private int port;
private String query;
private Universe universe;
private String username;
private String password;

private final String DEFAULT_DB_USER = "yugabyte";
private final String DEFAULT_DB_PASSWORD = "yugabyte";
private final String hostName;
private final int port;
private final String query;
private final Universe universe;
private final String username;
private final String password;

public SlowQueryExecutor(
String hostName,
Expand All @@ -52,9 +46,8 @@ public SlowQueryExecutor(
this.port = port;
this.universe = universe;
this.query = query;
this.username = username == null ? DEFAULT_DB_USER : username;
this.password = password == null ? DEFAULT_DB_PASSWORD : password;
this.apiHelper = Play.current().injector().instanceOf(ApiHelper.class);
this.username = username == null ? "yugabyte" : username;
this.password = password == null ? "yugabyte" : password;
}

private List<Map<String, Object>> resultSetToMap(ResultSet result) throws SQLException {
Expand All @@ -81,8 +74,8 @@ public JsonNode call() {
ObjectNode response = Json.newObject();
String connectString = String.format("jdbc:postgresql://%s:%d/%s", hostName, port, "postgres");
Properties connInfo = new Properties();
connInfo.put("user", this.username == null ? DEFAULT_DB_USER : this.username);
connInfo.put("password", this.password == null ? DEFAULT_DB_PASSWORD : this.password);
connInfo.put("user", this.username);
connInfo.put("password", this.password);
UniverseDefinitionTaskParams.Cluster primaryCluster =
universe.getUniverseDetails().getPrimaryCluster();
if (primaryCluster.userIntent.enableClientToNodeEncrypt) {
Expand All @@ -92,19 +85,20 @@ public JsonNode call() {
try (Connection conn = DriverManager.getConnection(connectString, connInfo)) {
if (conn == null) {
response.put("error", "Unable to connect to DB");
response.put("type", "ysql");
} else {
PreparedStatement p = conn.prepareStatement(query);
boolean hasResult = p.execute();
if (hasResult) {

if (p.execute()) {
ResultSet result = p.getResultSet();
List<Map<String, Object>> rows = resultSetToMap(result);
response.put("result", toJson(rows));
response.set("result", toJson(rows));
}
}
} catch (SQLException e) {
response.put("error", e.getMessage());
} catch (Exception e) {
log.error(e.getMessage(), e);
response.put("error", e.getMessage());
response.put("type", "ysql");
}

return response;
Expand Down

0 comments on commit 0295e25

Please sign in to comment.