Skip to content

Commit

Permalink
[#23513] YSQL: Simplify several functions in ybc_pggate
Browse files Browse the repository at this point in the history
Summary:
Some of the functions in `ybc_pggate.cc` doesn't use `pgapi` object and C++ code. Such as:
- `YBCPgGetType`
- `YBCPgAllowForPrimaryKey`

It is not necessary to have them in ybc_pggate.

Some other functions return status and use out argument. But it is possible to return value directly:
- `YBCGetPgggateCurrentAllocatedBytes`
- `YbGetActualHeapSizeBytes`

Unused function was removed:
- `YBCPgInvalidateTableCacheByRelfileNodeId`
Jira: DB-12433

Test Plan: Jenkins

Reviewers: telgersma, amartsinchyk, pjain

Reviewed By: amartsinchyk

Subscribers: ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D37357
  • Loading branch information
d-uspenskiy committed Aug 21, 2024
1 parent e193fc6 commit 8178372
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 106 deletions.
8 changes: 5 additions & 3 deletions src/postgres/src/backend/catalog/yb_catalog/yb_type.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ YbDataTypeFromOidMod(int attnum, Oid type_id)

/* Find the type mapping entry */
const YBCPgTypeEntity *type_entity = YBCPgFindTypeEntity(type_id);
YBCPgDataType yb_type = YBCPgGetType(type_entity);
const YBCPgDataType yb_type =
type_entity ? type_entity->yb_type : YB_YQL_DATA_TYPE_UNKNOWN_DATA;

/* For non-primitive types, we need to look up the definition */
if (yb_type == YB_YQL_DATA_TYPE_UNKNOWN_DATA) {
Expand Down Expand Up @@ -215,8 +216,9 @@ Oid YbGetPrimitiveTypeOid(Oid type_id, char typtype, Oid typbasetype) {
bool
YbDataTypeIsValidForKey(Oid type_id)
{
const YBCPgTypeEntity *type_entity = YbDataTypeFromOidMod(InvalidAttrNumber, type_id);
return YBCPgAllowForPrimaryKey(type_entity);
const YBCPgTypeEntity *entity =
YbDataTypeFromOidMod(InvalidAttrNumber, type_id);
return entity && entity->allow_for_primary_key;
}

const YBCPgTypeEntity *
Expand Down
2 changes: 1 addition & 1 deletion src/postgres/src/backend/utils/mmgr/mcxt.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static void
YbPgMemUpdateCur()
{
#if YB_TCMALLOC_ENABLED
YbGetActualHeapSizeBytes(&PgMemTracker.backend_cur_allocated_mem_bytes);
PgMemTracker.backend_cur_allocated_mem_bytes = YBCGetActualHeapSizeBytes();
yb_pgstat_report_allocated_mem_bytes();
#endif
}
Expand Down
87 changes: 36 additions & 51 deletions src/yb/util/mem_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include <boost/range/algorithm_ext/erase.hpp>

#include "yb/gutil/map-util.h"
#include "yb/gutil/once.h"
#include "yb/gutil/strings/human_readable.h"
#include "yb/gutil/strings/substitute.h"

Expand Down Expand Up @@ -134,10 +133,6 @@ using strings::Substitute;

namespace {

// The ancestor for all trackers. Every tracker is visible from the root down.
shared_ptr<MemTracker> root_tracker;
GoogleOnceType root_tracker_once = GOOGLE_ONCE_INIT;

// Total amount of memory from calls to Release() since the last GC. If this
// is greater than mem_tracker_tcmalloc_gc_release_bytes, this will trigger a tcmalloc gc.
Atomic64 released_memory_since_gc;
Expand Down Expand Up @@ -186,6 +181,40 @@ std::string CreateMetricDescription(const MemTracker& mem_tracker) {
return CreateMetricLabel(mem_tracker);
}

std::shared_ptr<MemTracker> CreateRootTracker() {
int64_t limit = FLAGS_memory_limit_hard_bytes;
if (limit == 0) {
// If no limit is provided, we'll use
// - 85% of the RAM for tservers.
// - 10% of the RAM for masters.
int64_t total_ram;
CHECK_OK(Env::Default()->GetTotalRAMBytes(&total_ram));
DCHECK(FLAGS_default_memory_limit_to_ram_ratio != USE_RECOMMENDED_MEMORY_VALUE);
limit = total_ram * FLAGS_default_memory_limit_to_ram_ratio;
}

ConsumptionFunctor consumption_functor;

#if YB_TCMALLOC_ENABLED
consumption_functor = &GetTCMallocActualHeapSizeBytes;

if (FLAGS_mem_tracker_tcmalloc_gc_release_bytes < 0) {
// Allocate 1% of memory to the tcmalloc page heap freelist.
// On a 4GB RAM machine, the master gets 10%, so 400MB, so 1% is 4MB.
// On a 16GB RAM machine, the tserver gets 85%, so 13.6GB, so 1% is 136MB, so cap at 128MB.
FLAGS_mem_tracker_tcmalloc_gc_release_bytes =
std::min(static_cast<size_t>(1.0 * limit / 100), 128_MB);
}

LOG(INFO) << "Creating root MemTracker with garbage collection threshold "
<< FLAGS_mem_tracker_tcmalloc_gc_release_bytes << " bytes";
#endif

LOG(INFO) << "Root memory limit is " << limit;
return std::make_shared<MemTracker>(
limit, "root", std::move(consumption_functor), nullptr /* parent */, AddToParent::kFalse,
CreateMetrics::kFalse, std::string() /* metric_name */, IsRootTracker::kTrue);
}

} // namespace

Expand Down Expand Up @@ -233,45 +262,6 @@ void MemTracker::ConfigureTCMalloc() {
RegisterTCMallocTraceHooks();
}

void MemTracker::InitRootTrackerOnce() {
GoogleOnceInit(&root_tracker_once, &MemTracker::CreateRootTracker);
}

void MemTracker::CreateRootTracker() {
int64_t limit = FLAGS_memory_limit_hard_bytes;
if (limit == 0) {
// If no limit is provided, we'll use
// - 85% of the RAM for tservers.
// - 10% of the RAM for masters.
int64_t total_ram;
CHECK_OK(Env::Default()->GetTotalRAMBytes(&total_ram));
DCHECK(FLAGS_default_memory_limit_to_ram_ratio != USE_RECOMMENDED_MEMORY_VALUE);
limit = total_ram * FLAGS_default_memory_limit_to_ram_ratio;
}

ConsumptionFunctor consumption_functor;

#if YB_TCMALLOC_ENABLED
consumption_functor = &GetTCMallocActualHeapSizeBytes;

if (FLAGS_mem_tracker_tcmalloc_gc_release_bytes < 0) {
// Allocate 1% of memory to the tcmalloc page heap freelist.
// On a 4GB RAM machine, the master gets 10%, so 400MB, so 1% is 4MB.
// On a 16GB RAM machine, the tserver gets 85%, so 13.6GB, so 1% is 136MB, so cap at 128MB.
FLAGS_mem_tracker_tcmalloc_gc_release_bytes =
std::min(static_cast<size_t>(1.0 * limit / 100), 128_MB);
}

LOG(INFO) << "Creating root MemTracker with garbage collection threshold "
<< FLAGS_mem_tracker_tcmalloc_gc_release_bytes << " bytes";
#endif

LOG(INFO) << "Root memory limit is " << limit;
root_tracker = std::make_shared<MemTracker>(
limit, "root", std::move(consumption_functor), nullptr /* parent */, AddToParent::kFalse,
CreateMetrics::kFalse, std::string() /* metric_name */, IsRootTracker::kTrue);
}

shared_ptr<MemTracker> MemTracker::CreateTracker(int64_t byte_limit,
const string& id,
const std::string& metric_name,
Expand Down Expand Up @@ -818,13 +808,8 @@ void MemTracker::LogUpdate(bool is_consume, int64_t bytes) const {
LOG(ERROR) << ss.str();
}

int64_t MemTracker::GetRootTrackerConsumption() {
InitRootTrackerOnce();
return root_tracker->consumption();
}

shared_ptr<MemTracker> MemTracker::GetRootTracker() {
InitRootTrackerOnce();
const shared_ptr<MemTracker>& MemTracker::GetRootTracker() {
static auto root_tracker = CreateRootTracker();
return root_tracker;
}

Expand Down
11 changes: 1 addition & 10 deletions src/yb/util/mem_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,7 @@ class MemTracker : public std::enable_shared_from_this<MemTracker> {
static std::vector<MemTrackerPtr> ListTrackers();

// Gets a shared_ptr to the "root" tracker, creating it if necessary.
static MemTrackerPtr GetRootTracker();

// Get the memory consumption from the "root" tracker, creating it if necessary.
static int64_t GetRootTrackerConsumption();
static const MemTrackerPtr& GetRootTracker();

static uint64_t GetTrackedMemory();

Expand Down Expand Up @@ -477,12 +474,6 @@ class MemTracker : public std::enable_shared_from_this<MemTracker> {
// 2. Must be called with parent->child_trackers_lock_ held.
MemTrackerPtr FindChildUnlocked(const std::string& id);

// Creates the root tracker.
static void CreateRootTracker();

// Tracks state to ensure that CreateRootTracker is only called once
static void InitRootTrackerOnce();

const int64_t limit_;
const int64_t soft_limit_;
const std::string id_;
Expand Down
7 changes: 2 additions & 5 deletions src/yb/yql/pggate/pggate.h
Original file line number Diff line number Diff line change
Expand Up @@ -754,12 +754,9 @@ class PgApiImpl {
const PgObjectId& table_id, Slice lower_bound_key, Slice upper_bound_key,
uint64_t max_num_ranges, uint64_t range_size_bytes, bool is_forward, uint32_t max_key_length);

MemTracker &GetMemTracker() { return *mem_tracker_; }
MemTracker& GetMemTracker() { return *mem_tracker_; }

MemTracker &GetRootMemTracker() { return *MemTracker::GetRootTracker(); }

// Using this function instead of GetRootMemTracker allows us to avoid copying a shared_pointer
int64_t GetRootMemTrackerConsumption() { return MemTracker::GetRootTrackerConsumption(); }
MemTracker& GetRootMemTracker() { return *MemTracker::GetRootTracker(); }

void DumpSessionState(YBCPgSessionState* session_data);

Expand Down
3 changes: 0 additions & 3 deletions src/yb/yql/pggate/ybc_pg_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,7 @@ typedef enum TxnPriorityRequirement {
kHighestPriority
} TxnPriorityRequirement;

// API to read type information.
const YBCPgTypeEntity *YBCPgFindTypeEntity(int type_oid);
YBCPgDataType YBCPgGetType(const YBCPgTypeEntity *type_entity);
bool YBCPgAllowForPrimaryKey(const YBCPgTypeEntity *type_entity);

// PostgreSQL can represent text strings up to 1 GB minus a four-byte header.
static const int64_t kYBCMaxPostgresTextSizeBytes = 1024ll * 1024 * 1024 - 4;
Expand Down
35 changes: 4 additions & 31 deletions src/yb/yql/pggate/ybc_pggate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -592,34 +592,12 @@ const YBCPgTypeEntity *YBCPgFindTypeEntity(int type_oid) {
return pgapi->FindTypeEntity(type_oid);
}

YBCPgDataType YBCPgGetType(const YBCPgTypeEntity *type_entity) {
if (type_entity) {
return type_entity->yb_type;
}
return YB_YQL_DATA_TYPE_UNKNOWN_DATA;
}

bool YBCPgAllowForPrimaryKey(const YBCPgTypeEntity *type_entity) {
if (type_entity) {
return type_entity->allow_for_primary_key;
}
return false;
}

YBCStatus YBCGetPgggateCurrentAllocatedBytes(int64_t *consumption) {
*consumption = GetTCMallocCurrentAllocatedBytes();
return YBCStatusOK();
int64_t YBCGetPgggateCurrentAllocatedBytes() {
return GetTCMallocCurrentAllocatedBytes();
}

YBCStatus YbGetActualHeapSizeBytes(int64_t *consumption) {
#ifdef YB_TCMALLOC_ENABLED
// Use GetRootMemTrackerConsumption instead of directly accessing TCMalloc to avoid excess
// calls to TCMalloc on every memory allocation.
*consumption = pgapi ? pgapi->GetRootMemTrackerConsumption() : 0;
#else
*consumption = 0;
#endif
return YBCStatusOK();
int64_t YBCGetActualHeapSizeBytes() {
return pgapi ? pgapi->GetRootMemTracker().consumption() : 0;
}

bool YBCTryMemConsume(int64_t bytes) {
Expand Down Expand Up @@ -856,11 +834,6 @@ YBCStatus YBCPgInvalidateTableCacheByTableId(const char *table_id) {
return YBCStatusOK();
}

void YBCPgInvalidateTableCacheByRelfileNodeId(const YBCPgOid database_oid,
const YBCPgOid table_oid) {
pgapi->InvalidateTableCache(PgObjectId(database_oid, table_oid));
}

// Tablegroup Operations ---------------------------------------------------------------------------

YBCStatus YBCPgNewCreateTablegroup(const char *database_name,
Expand Down
4 changes: 2 additions & 2 deletions src/yb/yql/pggate/ybc_pggate.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ const unsigned char* YBCGetLocalTserverUuid();
// Get access to callbacks.
const YBCPgCallbacks* YBCGetPgCallbacks();

YBCStatus YBCGetPgggateCurrentAllocatedBytes(int64_t *consumption);
int64_t YBCGetPgggateCurrentAllocatedBytes();

YBCStatus YbGetActualHeapSizeBytes(int64_t *consumption);
int64_t YBCGetActualHeapSizeBytes();

// Call root MemTacker to consume the consumption bytes.
// Return true if MemTracker exists (inited by pggate); otherwise false.
Expand Down

0 comments on commit 8178372

Please sign in to comment.