Skip to content

Commit

Permalink
[#22937] docdb: Backward scans: make pggate be aware of fast backward…
Browse files Browse the repository at this point in the history
… scan capability

Summary:
This is a first part of CBO-related changes for backward scans, which is not directly related to
the cost calculation. But the aim of the change is to make pggate be aware of fast backward scan
capability to be able to correctly identify the cost of backward scans in the CBO.
Refer to #22370 for the details.

For this purpose the flag is moved to common_flags.cc and YBCPgGFlagsAccessor is updated
to be able to get the value of the flag. Additionally the flag is mark as `NON_RUNTIME` to prevent
any confusion from a user side as PG flags does not support runtime change. Such change is
acceptable as it is not expected the flag change in future.

**Upgrade/Rollback safety:**
In the worst case, when the node is not yet upgraded, the planner/optimizer will not be aware of
the fast backward scan capability and as a result would be choosing a different execution path
without fast backward scan optimization avoiding possible performance gain. On the other hand,
if the planner/optimizer already aware of the capability (and it's on), but remote yb-tserver is
not yet upgraded, then it may result in a slower query but with no correctness impact.

Jira: DB-11853

Test Plan: Jenkins

Reviewers: sergei, amartsinchyk, rthallam

Reviewed By: amartsinchyk

Subscribers: hsunder, ybase, yql

Differential Revision: https://phorge.dev.yugabyte.com/D36245
  • Loading branch information
arybochkin committed Jul 15, 2024
1 parent cc63aaf commit 1773ae2
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 14 deletions.
6 changes: 6 additions & 0 deletions src/postgres/src/backend/utils/misc/pg_yb_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -4968,3 +4968,9 @@ YbReadTimePointHandle YbBuildCurrentReadTimePointHandle()
.has_value = true, .value = YBCPgGetCurrentReadTimePoint()}
: (YbReadTimePointHandle){};
}

// TODO(#22370): the method will be used to make Const Based Optimizer to be aware of
// fast backward scan capability.
bool YbUseFastBackwardScan() {
return *(YBCGetGFlags()->ysql_use_fast_backward_scan);
}
2 changes: 2 additions & 0 deletions src/postgres/src/include/pg_yb_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1154,4 +1154,6 @@ extern bool YbIsReadCommittedTxn();

extern YbReadTimePointHandle YbBuildCurrentReadTimePointHandle();

extern bool YbUseFastBackwardScan();

#endif /* PG_YB_UTILS_H */
16 changes: 11 additions & 5 deletions src/yb/common/common_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ DEFINE_RUNTIME_AUTO_bool(enable_automatic_tablet_splitting, kExternal, false, tr
"If false, disables automatic tablet splitting driven from the yb-master side.");

DEFINE_UNKNOWN_bool(log_ysql_catalog_versions, false,
"Log YSQL catalog events. For debugging purposes.");
"Log YSQL catalog events. For debugging purposes.");
TAG_FLAG(log_ysql_catalog_versions, hidden);

DEPRECATE_FLAG(bool, disable_hybrid_scan, "11_2022");
Expand Down Expand Up @@ -124,7 +124,7 @@ DEFINE_RUNTIME_AUTO_PG_FLAG(
"Requires yb_enable_replication_commands to be true.");

DEFINE_NON_RUNTIME_bool(TEST_ysql_hide_catalog_version_increment_log, false,
"Hide catalog version increment log messages.");
"Hide catalog version increment log messages.");
TAG_FLAG(TEST_ysql_hide_catalog_version_increment_log, hidden);

// The following flags related to the cloud, region and availability zone that an instance is
Expand All @@ -137,11 +137,12 @@ TAG_FLAG(TEST_ysql_hide_catalog_version_increment_log, hidden);
// These are currently for use in a cloud-based deployment, but could be retrofitted to work for
// an on-premise deployment as well, with datacenter, cluster and rack levels, for example.
DEFINE_NON_RUNTIME_string(placement_cloud, "cloud1",
"The cloud in which this instance is started.");
"The cloud in which this instance is started.");
DEFINE_NON_RUNTIME_string(placement_region, "datacenter1",
"The cloud region in which this instance is started.");
"The cloud region in which this instance is started.");
DEFINE_NON_RUNTIME_string(placement_zone, "rack1",
"The cloud availability zone in which this instance is started.");
"The cloud availability zone in which this instance is started.");

namespace {

constexpr const auto kMinRpcThrottleThresholdBytes = 16;
Expand Down Expand Up @@ -203,6 +204,11 @@ DEFINE_NON_RUNTIME_int32(cdc_read_rpc_timeout_ms, 30 * 1000,
"Timeout used for CDC read rpc calls. Reads normally occur cross-cluster.");
TAG_FLAG(cdc_read_rpc_timeout_ms, advanced);

// The flag is used both in DocRowwiseIterator and at PG side (needed for Cost Based Optimizer).
// But it is not tagged with kPg as it would be used both for YSQL and YCQL (refer to GH #22371).
DEFINE_NON_RUNTIME_PREVIEW_bool(use_fast_backward_scan, false,
"Use backward scan optimization to build a row in the reverse order for YSQL.");

namespace yb {

void InitCommonFlags() {
Expand Down
10 changes: 3 additions & 7 deletions src/yb/docdb/doc_rowwise_iterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,14 @@
#include "yb/util/status_log.h"
#include "yb/util/strongly_typed_bool.h"

using std::string;

DEFINE_RUNTIME_bool(ysql_use_flat_doc_reader, true,
"Use DocDBTableReader optimization that relies on having at most 1 subkey for YSQL.");

DEFINE_RUNTIME_PREVIEW_bool(use_fast_backward_scan, false,
"Use backward scan optimization to build a row in the reverse order. "
"Applicable for YSQL flat doc reader only.");

DEFINE_test_flag(int32, fetch_next_delay_ms, 0, "Amount of time to delay inside FetchNext");
DEFINE_test_flag(string, fetch_next_delay_column, "", "Only delay when schema has specific column");

DECLARE_bool(use_fast_backward_scan);

using namespace std::chrono_literals;

namespace yb::docdb {
Expand Down Expand Up @@ -381,7 +377,7 @@ Status DocRowwiseIterator::FillRow(QLTableRowPair out) {
return FillRow(out.table_row, out.projection);
}

string DocRowwiseIterator::ToString() const {
std::string DocRowwiseIterator::ToString() const {
return "DocRowwiseIterator";
}

Expand Down
2 changes: 1 addition & 1 deletion src/yb/docdb/docrowwiseiterator-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ Result<std::string> ConvertIteratorRowsToString(
buffer << VERIFY_RESULT(QLTableRowToString(schema, row, projection)) << std::endl;
}
} else {
// TODO(#22371): FLAGS_use_fast_backward_scan should not be set while the iterator creation.
// TODO(#22371): FLAGS_use_fast_backward_scan must not be set for this case.
down_cast<docdb::DocRowwiseIterator*>(iter)->TEST_force_allow_fetch_pg_table_row();
dockv::ReaderProjection reader_projection(projection ? *projection : schema);
dockv::PgTableRow row(reader_projection);
Expand Down
2 changes: 1 addition & 1 deletion src/yb/docdb/intent_aware_iterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ IntentAwareIterator::IntentAwareIterator(
transaction_status_cache_(
txn_op_context_, read_operation_data.read_time, read_operation_data.deadline) {
VTRACE(1, __func__);
VLOG(4) << "IntentAwareIterator, read_operation_data: " << read_operation_data.ToString()
VLOG(2) << "IntentAwareIterator, read_operation_data: " << read_operation_data.ToString()
<< ", txn_op_context: " << txn_op_context_ << ", " << TRACE_BOUNDS
<< ", use_fast_backward_scan: " << use_fast_backward_scan;

Expand Down
1 change: 1 addition & 0 deletions src/yb/yql/pggate/ybc_pg_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ typedef struct PgGFlagsAccessor {
const bool* ysql_enable_pg_per_database_oid_allocator;
const bool* ysql_enable_db_catalog_version_mode;
const bool* TEST_ysql_hide_catalog_version_increment_log;
const bool* ysql_use_fast_backward_scan;
} YBCPgGFlagsAccessor;

typedef struct YbTablePropertiesData {
Expand Down
3 changes: 3 additions & 0 deletions src/yb/yql/pggate/ybc_pggate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ DEFINE_NON_RUNTIME_bool(

DECLARE_bool(TEST_ash_debug_aux);

DECLARE_bool(use_fast_backward_scan);

namespace {

bool PreloadAdditionalCatalogListValidator(const char* flag_name, const std::string& flag_val) {
Expand Down Expand Up @@ -1924,6 +1926,7 @@ const YBCPgGFlagsAccessor* YBCGetGFlags() {
&FLAGS_ysql_enable_db_catalog_version_mode,
.TEST_ysql_hide_catalog_version_increment_log =
&FLAGS_TEST_ysql_hide_catalog_version_increment_log,
.ysql_use_fast_backward_scan = &FLAGS_use_fast_backward_scan,
};
// clang-format on
return &accessor;
Expand Down
3 changes: 3 additions & 0 deletions src/yb/yql/pgwrapper/pg_single_tserver-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -772,8 +772,11 @@ class PgFastBackwardScanTest
Status FetchAndValidate(
PGConn& conn, const std::string& fetch_stmt,
const std::string& expected_result, size_t num_iterations = 4) {
VLOG_WITH_FUNC(1)
<< "fetch stmt: '" << fetch_stmt << "' expected result: '" << expected_result << "'";
for ([[maybe_unused]] const auto _ : Range(num_iterations)) {
const auto result = VERIFY_RESULT(conn.FetchAllAsString(fetch_stmt));
VLOG_WITH_FUNC(1) << "iteration: " << _ << " actual result: '" << result << "'";
SCHECK_EQ(result, expected_result, IllegalState, "Unexpected result");
}
return Status::OK();
Expand Down

0 comments on commit 1773ae2

Please sign in to comment.