Skip to content

Use BRAVO lock for g_records_rwlock #9395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
masaori335 wants to merge 1 commit into from
Closed

Use BRAVO lock for g_records_rwlock #9395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/records/P_RecCore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "tscore/ink_thread.h"
#include "tscore/ink_rwlock.h"
#include "tscore/TextBuffer.h"
#include "tscpp/util/Bravo.h"

#include "I_RecCore.h"
#include "P_RecDefs.h"
Expand All @@ -39,7 +40,7 @@
// records, record hash-table, and hash-table rwlock
extern RecRecord *g_records;
extern std::unordered_map<std::string, RecRecord *> g_records_ht;
extern ink_rwlock g_records_rwlock;
extern ts::bravo::shared_mutex g_records_rwlock;
extern int g_num_records;

// records.yaml items
Expand Down
24 changes: 9 additions & 15 deletions src/records/P_RecCore.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ RecSetRecord(RecT rec_type, const char *name, RecDataT data_type, RecData *data,
// FIXME: Most of the time we set, we don't actually need to wrlock
// since we are not modifying the g_records_ht.
if (lock) {
ink_rwlock_wrlock(&g_records_rwlock);
g_records_rwlock.lock();
}
if (auto it = g_records_ht.find(name); it != g_records_ht.end()) {
r1 = it->second;
Expand Down Expand Up @@ -199,7 +199,7 @@ RecSetRecord(RecT rec_type, const char *name, RecDataT data_type, RecData *data,

Ldone:
if (lock) {
ink_rwlock_unlock(&g_records_rwlock);
g_records_rwlock.unlock();
}

return err;
Expand Down Expand Up @@ -279,7 +279,7 @@ RecReadStatsFile()
ats_scoped_str snap_fpath(RecConfigReadPersistentStatsPath());

// lock our hash table
ink_rwlock_wrlock(&g_records_rwlock);
std::lock_guard lock(g_records_rwlock);

CheckSnapFileVersion(snap_fpath);

Expand Down Expand Up @@ -318,7 +318,6 @@ RecReadStatsFile()
}
}

ink_rwlock_unlock(&g_records_rwlock);
ats_free(m);

return REC_ERR_OKAY;
Expand Down Expand Up @@ -380,14 +379,11 @@ RecReadConfigFile()
RecDebug(DL_Note, "Reading '%s'", g_rec_config_fpath);

// lock our hash table
ink_rwlock_wrlock(&g_records_rwlock);
std::lock_guard lock(g_records_rwlock);

// Parse the actual file and hash the values.
RecConfigFileParse(g_rec_config_fpath, RecConsumeConfigEntry);

// release our hash table
ink_rwlock_unlock(&g_records_rwlock);

return REC_ERR_OKAY;
}

Expand All @@ -397,12 +393,12 @@ RecReadYamlConfigFile()
RecDebug(DL_Debug, "Reading '%s'", g_rec_config_fpath);

// lock our hash table
ink_rwlock_wrlock(&g_records_rwlock); // review this lock maybe it should be done inside the API
// review this lock maybe it should be done inside the API
std::lock_guard lock(g_records_rwlock);

// Parse the actual file and hash the values.
auto ret = RecYAMLConfigFileParse(g_rec_config_fpath, SetRecordFromYAMLNode);

ink_rwlock_unlock(&g_records_rwlock);
return ret;
}

Expand All @@ -416,7 +412,7 @@ RecExecConfigUpdateCbs(unsigned int update_required_type)
int i, num_records;
RecUpdateT update_type = RECU_NULL;

ink_rwlock_rdlock(&g_records_rwlock);
ts::bravo::shared_lock lock(g_records_rwlock);

num_records = g_num_records;
for (i = 0; i < num_records; i++) {
Expand Down Expand Up @@ -450,8 +446,6 @@ RecExecConfigUpdateCbs(unsigned int update_required_type)
rec_mutex_release(&(r->lock));
}

ink_rwlock_unlock(&g_records_rwlock);

return update_type;
}

Expand Down Expand Up @@ -527,7 +521,7 @@ RecSetSyncRequired(char *name, bool lock)
// FIXME: Most of the time we set, we don't actually need to wrlock
// since we are not modifying the g_records_ht.
if (lock) {
ink_rwlock_wrlock(&g_records_rwlock);
g_records_rwlock.lock();
}
if (auto it = g_records_ht.find(name); it != g_records_ht.end()) {
r1 = it->second;
Expand All @@ -543,7 +537,7 @@ RecSetSyncRequired(char *name, bool lock)
}

if (lock) {
ink_rwlock_unlock(&g_records_rwlock);
g_records_rwlock.unlock();
}

return err;
Expand Down
Loading