Skip to content

Commit

Permalink
[pg15] test: add appendToYsqlPgConf for Java tests
Browse files Browse the repository at this point in the history
Summary:
Since issue #22711 is still open, add a workaround for Java tests to be
able to append to ysql_pg_conf_csv.  This is needed for future work to
set yb_use_hash_splitting_by_default to false for ported regress tests
and to set compute_query_id to regress for all regress tests.  The
method of implementation I chose does not prevent a developer from
forgetting to use this helper and accidentally overwriting the entire
ysql_pg_conf_csv.  Given the current framework, it is difficult to do
that.

Test Plan:
Depends on D36303
Jenkins: rebase: pg15

Reviewers: aagrawal, fizaa, tfoucher

Reviewed By: tfoucher

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D36306
  • Loading branch information
jaki committed Jul 2, 2024
1 parent 4f17a04 commit 6e887a9
Show file tree
Hide file tree
Showing 21 changed files with 49 additions and 32 deletions.
33 changes: 21 additions & 12 deletions java/yb-pgsql/src/test/java/org/yb/pgsql/BasePgSQLTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,6 @@ public class BasePgSQLTest extends BaseMiniClusterTest {

protected static final int DEFAULT_STATEMENT_TIMEOUT_MS = 30000;

protected static Map<String, String> FailOnConflictTestGflags = new HashMap<String, String>()
{
{
put("enable_wait_queues", "false");
// The retries are set to 2 to speed up the tests.
put("ysql_pg_conf_csv", maxQueryLayerRetriesConf(2));
}
};

protected static ConcurrentSkipListSet<Integer> stuckBackendPidsConcMap =
new ConcurrentSkipListSet<>();

Expand Down Expand Up @@ -236,6 +227,26 @@ protected Integer getYsqlRequestLimit() {
return null;
}

/**
* Add ysql_pg_conf_csv flag values using this method to avoid clobbering existing values.
* @param flagMap the map of flags to mutate
* @param value the raw text to append to the ysql_pg_conf_csv flag value
*/
protected static void appendToYsqlPgConf(Map<String, String> flagMap, String value) {
final String flagName = "ysql_pg_conf_csv";
if (flagMap.containsKey(flagName)) {
flagMap.put(flagName, flagMap.get(flagName) + "," + value);
} else {
flagMap.put(flagName, value);
}
}

protected static void setFailOnConflictFlags(Map<String, String> flagMap) {
flagMap.put("enable_wait_queues", "false");
// The retries are set to 2 to speed up the tests.
appendToYsqlPgConf(flagMap, maxQueryLayerRetriesConf(2));
}

/**
* @return flags shared between tablet server and initdb
*/
Expand Down Expand Up @@ -2184,9 +2195,7 @@ protected long getNumDocdbRequests(Statement stmt, String query) throws Exceptio

/** Creates a new tserver and returns its id. **/
protected int spawnTServer() throws Exception {
int tserverId = miniCluster.getNumTServers();
miniCluster.startTServer(getTServerFlags());
return tserverId;
return spawnTServerWithFlags(new HashMap<String, String>());
}

/** Creates a new tserver with additional flags and returns its id. **/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected Map<String, String> getTServerFlags() {
// This test expects one of two conflicting transactions to be killed immediately in each
// iteration. Therefore, we run it in fail-on-conflict concurrency control.
// TODO(wait-queues): https://github.com/yugabyte/yugabyte-db/issues/17871
flagMap.putAll(FailOnConflictTestGflags);
setFailOnConflictFlags(flagMap);
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
flagMap.put("ysql_prefetch_limit", "1024");
flagMap.put("ysql_session_max_batch_size", "512");
flagMap.put("ysql_pg_conf_csv", "\"shared_preload_libraries=auto_explain\"");
appendToYsqlPgConf(flagMap, "shared_preload_libraries=auto_explain");
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ protected Map<String, String> getTServerFlags() {
flagMap.put("yb_enable_read_committed_isolation", "true");
// This test depends on fail-on-conflict concurrency control to perform its validation.
// TODO(wait-queues): https://github.com/yugabyte/yugabyte-db/issues/17871
flagMap.putAll(FailOnConflictTestGflags);
setFailOnConflictFlags(flagMap);
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class TestPgFastpathIntentdbSeeks extends BasePgSQLTestWithRpcMetric {
protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
flagMap.put("enable_wait_queues", "false");
flagMap.put("ysql_pg_conf_csv", maxQueryLayerRetriesConf(0));
appendToYsqlPgConf(flagMap, maxQueryLayerRetriesConf(0));
// Verbose logging of intentsdb seeks/postgres statements
flagMap.put("vmodule", "docdb=4,conflict_resolution=5");
flagMap.put("ysql_log_statement", "all");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
// This test depends on fail-on-conflict concurrency control to perform its validation.
// TODO(wait-queues): https://github.com/yugabyte/yugabyte-db/issues/17871
flagMap.putAll(FailOnConflictTestGflags);
setFailOnConflictFlags(flagMap);
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf", TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
appendToYsqlPgConf(flags, TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
return flags;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
flagMap.put("ysql_pg_conf", "shared_preload_libraries='passwordcheck'");
appendToYsqlPgConf(flagMap, "shared_preload_libraries='passwordcheck'");
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public int getTestMethodTimeoutSec() {
protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
flagMap.put("ysql_sequence_cache_minval", Integer.toString(TURN_OFF_SEQUENCE_CACHE_FLAG));
flagMap.put("ysql_pg_conf", TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
appendToYsqlPgConf(flagMap, TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
if(!TestUtils.isReleaseBuild()){
flagMap.put("yb_client_admin_operation_timeout_sec", Integer.toString(240));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
// This test depends on fail-on-conflict concurrency control to perform its validation.
// TODO(wait-queues): https://github.com/yugabyte/yugabyte-db/issues/17871
flagMap.putAll(FailOnConflictTestGflags);
setFailOnConflictFlags(flagMap);
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf", TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
appendToYsqlPgConf(flags, TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
return flags;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf", TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
appendToYsqlPgConf(flags, TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
flags.put("yb_enable_read_committed_isolation", "true");
return flags;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf", TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
appendToYsqlPgConf(flags, TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
flags.put("yb_num_shards_per_tserver", Integer.toString(1));
flags.put("db_write_buffer_size", Integer.toString(122880));
flags.put("db_block_size_bytes", Integer.toString(2048));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
flagMap.put("ysql_pg_conf", "shared_preload_libraries='pg_stat_monitor'");
appendToYsqlPgConf(flagMap, "shared_preload_libraries='pg_stat_monitor'");
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf", TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
appendToYsqlPgConf(flags, TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
return flags;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public int getTestMethodTimeoutSec() {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf", TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
appendToYsqlPgConf(flags, TURN_OFF_COPY_FROM_BATCH_TRANSACTION);
return flags;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected Map<String, String> getTServerFlags() {
* Setting yb_max_query_layer_retries allows to reliably test wait queue semantics in
* isolation by avoiding query layer retries of serialization errors.
*/
flagMap.put("ysql_pg_conf_csv", maxQueryLayerRetriesConf(0));
appendToYsqlPgConf(flagMap, maxQueryLayerRetriesConf(0));
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private void checkTransactionFairness(
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf_csv", maxQueryLayerRetriesConf(2));
appendToYsqlPgConf(flags, maxQueryLayerRetriesConf(2));
return flags;
}

Expand All @@ -93,6 +93,14 @@ void runWithFailOnConflict() throws Exception {
// Some of these tests depend on fail-on-conflict concurrency control to perform its validation.
// TODO(wait-queues): https://github.com/yugabyte/yugabyte-db/issues/17871
markClusterNeedsRecreation();
Map<String, String> FailOnConflictTestGflags = new TreeMap<String, String>()
{
{
put("enable_wait_queues", "false");
// The retries are set to 2 to speed up the tests.
put("ysql_pg_conf_csv", maxQueryLayerRetriesConf(2));
}
};
restartClusterWithFlags(FailOnConflictTestGflags, FailOnConflictTestGflags);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected Map<String, String> getTServerFlags() {
flags.put("TEST_transactional_read_delay_ms", "100");
// This test depends on fail-on-conflict concurrency control to perform its validation.
// TODO(wait-queues): https://github.com/yugabyte/yugabyte-db/issues/17871
flags.putAll(FailOnConflictTestGflags);
setFailOnConflictFlags(flags);
return flags;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class TestStatementsQtext extends BasePgSQLTest {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flagMap = super.getTServerFlags();
flagMap.put("ysql_pg_conf_csv", "pg_stat_statements.yb_qtext_size_limit=1kB");
appendToYsqlPgConf(flagMap, "pg_stat_statements.yb_qtext_size_limit=1kB");
return flagMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public abstract class TestToastBase extends BasePgSQLTest {
@Override
protected Map<String, String> getTServerFlags() {
Map<String, String> flags = super.getTServerFlags();
flags.put("ysql_pg_conf_csv", "max_stack_depth='7680kB'");
appendToYsqlPgConf(flags, "max_stack_depth='7680kB'");
return flags;
}

Expand Down

0 comments on commit 6e887a9

Please sign in to comment.