Skip to content

Commit

Permalink
[#566] Enable TIMESTAMP type in PostgreSQL; fix float test on ASAN by…
Browse files Browse the repository at this point in the history
… disabling UBSAN on float.c

Summary:
Enable TIMESTAMP type in PostgreSQL. The serialization format is a 64-bit integer.
Also fix `yb_float4` and `yb_float8` tests on ASAN by disabling UBSAN in `float.c`.

PostgreSQL test framework improvement: display a human-readable side-by-side diff with line numbers
specified separately on each side. Example: http://bit.ly/2QuYYuO

This commit fixes #566.

Test Plan: Jenkins

Reviewers: neil, robert, mihnea

Reviewed By: mihnea

Subscribers: bharat, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D5687
  • Loading branch information
mbautin committed Nov 10, 2018
1 parent 5f489b7 commit b58c985
Show file tree
Hide file tree
Showing 25 changed files with 2,238 additions and 194 deletions.
26 changes: 18 additions & 8 deletions build-support/compiler-wrappers/compiler-wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,21 @@ readonly GENERATED_BUILD_DEBUG_SCRIPT_DIR=$HOME/.yb-build-debug-scripts
readonly SCRIPT_NAME="compiler-wrapper.sh"
declare -i -r MAX_INPUT_FILES_TO_SHOW=20

# Files such as numeric.c, int.c, int8.c, and float.c get compiled with UBSAN (Undefined Behavior
# Sanitizer) turned off due to large number of overflow cases in them (by design).

# numeric.c is compiled without UBSAN as a fix for a problem observed with Clang 5.0 and 6.0:
# undefined reference to `__muloti4'
# (full log at http://bit.ly/2lYdYnp).
# Related to the problem reported at http://bit.ly/2NvS6MR.

# int.c and int8.c have a lot of assumptions regarding the exact resulting values of overflowing
# operations, and these assumptions are not true in UBSAN mode.

# float.c compiled in UBSAN mode causes the following error on the yb_float4 test (server process
# crashes): http://bit.ly/2AW9oye

readonly NO_UBSAN_RE='(numeric|int|int8|float)'

# -------------------------------------------------------------------------------------------------
# Common functions
Expand Down Expand Up @@ -453,14 +468,9 @@ local_build_exit_handler() {

if [[ ${build_type:-} == "asan" &&
$PWD == */postgres_build/src/backend/utils/adt &&
( $compiler_args_str == *\ -c\ -o\ numeric.o\ numeric.c\ * ||
$compiler_args_str == *\ -c\ -o\ int.o\ int.c\ * ||
$compiler_args_str == *\ -c\ -o\ int8.o\ int8.c\ * ) ]]; then
# A hack for the problem observed in ASAN when compiling numeric.c with Clang 5.0 and 6.0.
# undefined reference to `__muloti4'
# (full log at http://bit.ly/2lYdYnp).
# Related to the problem reported at http://bit.ly/2NvS6MR.

# Turn off UBSAN instrumentation in a number of PostgreSQL source files determined by the
# $NO_UBSAN_RE regular expression. See the definition of NO_UBSAN_RE for details.
$compiler_args_str =~ .*\ -c\ -o\ $NO_UBSAN_RE[.]o\ $NO_UBSAN_RE[.]c\ .* ]]; then
rewritten_args=()
for arg in "${compiler_args[@]}"; do
case $arg in
Expand Down
15 changes: 15 additions & 0 deletions java/yb-client/src/test/java/org/yb/client/TestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -466,4 +466,19 @@ public static void resetDefaultStdOutAndErr() {
System.setErr(defaultStdErr);
}

/**
* @param arr integer parameters
* @return the first of the given numbers that is positive
*/
public static int getFirstPositiveNumber(int... arr) {
for (int value : arr) {
if (value > 0)
return value;
}
if (arr.length > 0) {
return arr[arr.length - 1];
}
throw new IllegalArgumentException("No numbers given to firstPositiveNumber");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,13 @@ public class BaseMiniClusterTest extends BaseYBTest {

private static final Logger LOG = LoggerFactory.getLogger(BaseMiniClusterTest.class);

protected static final String NUM_MASTERS_PROP = "NUM_MASTERS";
// TODO: review the usage of the constants below.
protected static final int NUM_MASTERS = 3;
protected static final int NUM_TABLET_SERVERS = 3;
protected static final int DEFAULT_NUM_MASTERS = 3;

protected static final int STANDARD_DEVIATION_FACTOR = 2;
protected static final int DEFAULT_TIMEOUT_MS = 50000;

// Number of masters that will be started for this test if we're starting
// a cluster.
protected static final int NUM_MASTERS =
Integer.getInteger(NUM_MASTERS_PROP, DEFAULT_NUM_MASTERS);

/**
* This is used as the default timeout when calling YB Java client's async API.
*/
Expand All @@ -62,6 +58,18 @@ public class BaseMiniClusterTest extends BaseYBTest {
protected static String masterAddresses;
protected static List<HostAndPort> masterHostPorts;

protected int getReplicationFactor() {
return -1;
}

protected int getInitialNumMasters() {
return -1;
}

protected int getInitialNumTServers() {
return -1;
}

// Subclasses can override this to set the number of shards per tablet server.
protected int overridableNumShardsPerTServer() {
return MiniYBCluster.DEFAULT_NUM_SHARDS_PER_TSERVER;
Expand Down Expand Up @@ -112,7 +120,13 @@ protected void createMiniCluster() throws Exception {
if (!miniClusterEnabled()) {
return;
}
createMiniCluster(NUM_MASTERS, NUM_TABLET_SERVERS);
final int replicationFactor = getReplicationFactor();
createMiniCluster(
TestUtils.getFirstPositiveNumber(
getInitialNumMasters(), replicationFactor, MiniYBCluster.DEFAULT_NUM_MASTERS),
TestUtils.getFirstPositiveNumber(
getInitialNumTServers(), replicationFactor, MiniYBCluster.DEFAULT_NUM_TSERVERS)
);
}

/**
Expand Down Expand Up @@ -141,6 +155,7 @@ public void createMiniCluster(int numMasters, List<List<String>> tserverArgs)
.tserverArgs(tserverArgs)
.numShardsPerTServer(overridableNumShardsPerTServer())
.useIpWithCertificate(useIpWithCertificate)
.replicationFactor(getReplicationFactor())
.build();
masterAddresses = miniCluster.getMasterAddresses();
masterHostPorts = miniCluster.getMasterHostPorts();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public class MiniYBCluster implements AutoCloseable {

public static final int DEFAULT_NUM_SHARDS_PER_TSERVER = 3;

public static final int DEFAULT_NUM_MASTERS = 3;
public static final int DEFAULT_NUM_TSERVERS = 3;

private int numShardsPerTserver;
Expand All @@ -150,6 +151,8 @@ public class MiniYBCluster implements AutoCloseable {
private static final long DAEMON_MEMORY_LIMIT_HARD_BYTES_NON_TSAN = 1024 * 1024 * 1024;
private static final long DAEMON_MEMORY_LIMIT_HARD_BYTES_TSAN = 512 * 1024 * 1024;

private int replicationFactor = -1;

/**
* Not to be invoked directly, but through a {@link MiniYBClusterBuilder}.
*/
Expand All @@ -160,11 +163,13 @@ public class MiniYBCluster implements AutoCloseable {
List<List<String>> tserverArgs,
int numShardsPerTserver,
String testClassName,
boolean useIpWithCertificate) throws Exception {
boolean useIpWithCertificate,
int replicationFactor) throws Exception {
this.defaultTimeoutMs = defaultTimeoutMs;
this.testClassName = testClassName;
this.numShardsPerTserver = numShardsPerTserver;
this.useIpWithCertificate = useIpWithCertificate;
this.replicationFactor = replicationFactor;

startCluster(numMasters, numTservers, masterArgs, tserverArgs);
startSyncClient();
Expand Down Expand Up @@ -215,6 +220,10 @@ private List<String> getCommonDaemonFlags() {

commonFlags.add("--yb_num_shards_per_tserver=" + numShardsPerTserver);

if (replicationFactor > 0) {
commonFlags.add("--replication_factor=" + replicationFactor);
}

return commonFlags;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class MiniYBClusterBuilder {
private List<String> masterArgs = null;
private List<List<String>> tserverArgs = null;
private String testClassName = null;
private int replicationFactor = -1;

public MiniYBClusterBuilder numMasters(int numMasters) {
this.numMasters = numMasters;
Expand Down Expand Up @@ -85,8 +86,16 @@ public MiniYBClusterBuilder testClassName(String testClassName) {
return this;
}

/**
* Sets the replication factor for the mini-cluster.
*/
public MiniYBClusterBuilder replicationFactor(int replicationFactor) {
this.replicationFactor = replicationFactor;
return this;
}

public MiniYBCluster build() throws Exception {
return new MiniYBCluster(numMasters, numTservers, defaultTimeoutMs, masterArgs, tserverArgs,
numShardsPerTServer, testClassName, useIpWithCertificate);
numShardsPerTServer, testClassName, useIpWithCertificate, replicationFactor);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ private void analyzeSystemLog() throws IOException {
"tail -%d '%s' | egrep -i -C %d '%s'", NUM_LAST_SYSLOG_LINES_TO_USE,
SYSLOG_PATH, SYSLOG_CONTEXT_NUM_LINES, regexStr));
cmdResult.logStderr();
if (cmdResult.stdoutLines.isEmpty()) {
if (cmdResult.getStdoutLines().isEmpty()) {
if (!terminatedNormally()) {
LOG.warn("Could not find anything in " + SYSLOG_PATH + " relevant to the " +
"disappearance of process: " + MiniYBDaemon.this);
}
} else {
LOG.warn("Potentially relevant lines from " + SYSLOG_PATH +
" for termination of " + this + ":\n" +
StringUtil.joinLinesForLogging(cmdResult.stdoutLines));
StringUtil.joinLinesForLogging(cmdResult.getStdoutLines()));
}
}

Expand All @@ -89,7 +89,7 @@ private void analyzeMemoryUsage() throws IOException {
int numMasters = 0;
int numTservers = 0;
List<String> masterTserverPsLines = new ArrayList<String>();
for (String line : cmdResult.stdoutLines) {
for (String line : cmdResult.getStdoutLines()) {
// Four parts: RSS, pid, executable path, arguments.
String[] items = line.split("\\s+", 4);
if (items.length < 4) {
Expand Down Expand Up @@ -125,7 +125,7 @@ private void analyzeMemoryUsage() throws IOException {
", num tserver processes: " + numTservers +
", total tserver memory usage (MB): " + (totalTserverRssKB / 1024) + "; " +
"ps output:\n" +
StringUtil.joinLinesForLogging(cmdResult.stdoutLines));
StringUtil.joinLinesForLogging(cmdResult.getStdoutLines()));
} else {
LOG.info("Did not find any yb-master/yb-tserver processes in 'ps' output");
}
Expand Down
13 changes: 9 additions & 4 deletions java/yb-client/src/test/java/org/yb/util/CommandResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
public class CommandResult {
private static final Logger LOG = LoggerFactory.getLogger(CommandResult.class);

public final String cmd;
public final int exitCode;
public final List<String> stdoutLines;
public final List<String> stderrLines;
private final String cmd;
private final int exitCode;
private final List<String> stdoutLines;
private final List<String> stderrLines;

public CommandResult(
String cmd, int exitCode, List<String> stdoutLines, List<String> stderrLines) {
Expand Down Expand Up @@ -92,4 +92,9 @@ public List<String> getStderrLines() {
return stderrLines;
}

@Override
public String toString() {
return "Result of running command {{ " + cmd + " }}: exit code " + exitCode;
}

}
6 changes: 5 additions & 1 deletion java/yb-client/src/test/java/org/yb/util/SanitizerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ public static int nonTsanVsTsan(int nonTsanValue, int tsanValue) {

/** @return a timeout multiplier to apply in tests based on the build type */
public static double getTimeoutMultiplier() {
return isTSAN() ? 3.0 : 1.0;
if (isTSAN())
return 3;
if (isASAN())
return 1.5;
return 1;
}

/**
Expand Down
Loading

0 comments on commit b58c985

Please sign in to comment.