Skip to content
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

Various XHA improvements #18

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Various XHA improvements #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb25181
com_*_lock: reduce critical section scope
edwintorok Feb 18, 2025
b276416
Update .gitignore with CodeChecker files
edwintorok Feb 21, 2025
3b50a2a
CA-407106: Fix build with clang: remove nested functions
edwintorok Feb 18, 2025
f73bcd6
Use pthread_cond_timewait instead of waking up every 100ms
edwintorok Feb 18, 2025
446f61e
Improve xha locking latency
edwintorok Feb 21, 2025
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ include/mtcerrno.h
scripts/generr
scripts/generr.o
scripts/ha_errnorc
compile_commands.json
.cache/
35 changes: 23 additions & 12 deletions daemon/com.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
MTC_U32 in_use;
MTC_U32 ref_count;
MTC_U32 checksum; // to detect modification by reader
pthread_mutex_t thread_id_record_table_mutex;
THREAD_ID_RECORD thread_id_record_table[THREAD_ID_RECORD_NUM];
} HA_COMMON_OBJECT;

Expand Down Expand Up @@ -536,6 +537,7 @@
clock_gettime(CLOCK_MONOTONIC, &ts);
now = tstoms(ts);

pthread_mutex_lock(&object->thread_id_record_table_mutex);
switch (lock_state) {
case LOCK_STATE_READER_ACQUIREING:
case LOCK_STATE_WRITER_ACQUIREING:
Expand All @@ -550,6 +552,7 @@
object->thread_id_record_table[i].lock_state = lock_state;
object->thread_id_record_table[i].thread_id = self;
object->thread_id_record_table[i].changed_time = now;
pthread_mutex_unlock(&object->thread_id_record_table_mutex);
return;
}
}
Expand All @@ -570,12 +573,14 @@
//
object->thread_id_record_table[i].lock_state = lock_state;
object->thread_id_record_table[i].changed_time = now;
pthread_mutex_unlock(&object->thread_id_record_table_mutex);
return;
}
}
log_message(MTC_LOG_WARNING, "COM: thread_id %d not found in thraed_id_record_table.\n", self);
break;
}
pthread_mutex_unlock(&object->thread_id_record_table_mutex);
assert(FALSE);
return ;
}
Expand Down Expand Up @@ -754,6 +759,13 @@
handle = (HA_COMMON_OBJECT_HANDLE_INTERNAL *) *object_handle;
object->ref_count ++;
handle->object->in_use++;
pthread_ret = pthread_mutex_init(&handle->object->thread_id_record_table_mutex, NULL);
if (pthread_ret != 0)
{
log_internal(MTC_LOG_ERR, "COM: (%s) pthread_mutex_init failed (sys %d).\n", __func__, pthread_ret);
ret = MTC_ERROR_COM_PTHREAD;
goto error_return;
}
set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIREING);
LEAVE_CS;

Expand All @@ -769,6 +781,7 @@

pthread_ret = pthread_rwlock_wrlock(&handle->object->rwlock);
if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE;


ENTER_CS;
set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIRED);
Expand Down Expand Up @@ -937,6 +950,7 @@
{
log_message(MTC_LOG_WARNING, "COM: pthread_rwlock_destroy failed (sys %d).\n", pthread_ret);
}
pthread_ret = pthread_mutex_destroy(&object->thread_id_record_table_mutex);

Check warning

Code scanning / CodeChecker

Value stored to 'pthread_ret' is never read Warning

Value stored to 'pthread_ret' is never read
free_object(object);
}
error_return:
Expand Down Expand Up @@ -1097,20 +1111,19 @@
HA_COMMON_OBJECT_HANDLE_INTERNAL *handle = object_handle;
int pthread_ret;

ENTER_CS;
if (!valid_object_handle(handle))
{
log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__);
assert(FALSE);
ret = MTC_ERROR_COM_INVALID_HANDLE;
goto error_return;
}
ENTER_CS;
handle->object->in_use++;
set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIREING);
LEAVE_CS;
set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIREING);
pthread_ret = pthread_rwlock_wrlock(&handle->object->rwlock);
if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE;
ENTER_CS;
set_thread_id_record(handle->object, LOCK_STATE_WRITER_ACQUIRED);
if (pthread_ret != 0)
{
Expand All @@ -1122,7 +1135,6 @@
*buffer = handle->object->buffer;

error_return:
LEAVE_CS;
if (ret != MTC_SUCCESS)
{
log_status(ret, NULL);
Expand Down Expand Up @@ -1157,7 +1169,6 @@
int pthread_ret;
HA_COMMON_OBJECT_CALLBACK_LIST_ITEM *c;

ENTER_CS;
if (!valid_object_handle(handle))
{
log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__);
Expand All @@ -1166,6 +1177,7 @@
goto error_return;
}

ENTER_CS;
for (c = handle->object->callback_list_head; c != NULL; c = c->next)
{
c->func(c->object_handle, handle->object->buffer);
Expand All @@ -1176,6 +1188,8 @@


handle->object->in_use--;
LEAVE_CS;

set_thread_id_record(handle->object, LOCK_STATE_NONE);
pthread_ret = pthread_rwlock_unlock(&handle->object->rwlock);
if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE;
Expand All @@ -1187,7 +1201,6 @@
}

error_return:
LEAVE_CS;
if (ret != MTC_SUCCESS)
{
log_status(ret, NULL);
Expand Down Expand Up @@ -1224,20 +1237,19 @@
HA_COMMON_OBJECT_HANDLE_INTERNAL *handle = object_handle;
int pthread_ret;

ENTER_CS;
if (!valid_object_handle(handle))
{
log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__);
assert(FALSE);
ret = MTC_ERROR_COM_INVALID_HANDLE;
goto error_return;
}
ENTER_CS;
handle->object->in_use++;
set_thread_id_record(handle->object, LOCK_STATE_READER_ACQUIREING);
LEAVE_CS;
set_thread_id_record(handle->object, LOCK_STATE_READER_ACQUIREING);
pthread_ret = pthread_rwlock_rdlock(&handle->object->rwlock);
if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE;
ENTER_CS;
set_thread_id_record(handle->object, LOCK_STATE_READER_ACQUIRED);
if (pthread_ret != 0)
{
Expand All @@ -1248,7 +1260,6 @@
*buffer = handle->object->buffer;

error_return:
LEAVE_CS;
if (ret != MTC_SUCCESS)
{
log_status(ret, NULL);
Expand Down Expand Up @@ -1281,7 +1292,6 @@
HA_COMMON_OBJECT_HANDLE_INTERNAL *handle = object_handle;
int pthread_ret;

ENTER_CS;
if (!valid_object_handle(handle))
{
log_internal(MTC_LOG_ERR, "COM: (%s) invalid handle.\n", __func__);
Expand All @@ -1290,10 +1300,12 @@
goto error_return;
}

ENTER_CS;
#ifndef NDEBUG
assert(handle->object->checksum == calc_checksum_object_buffer(handle->object));
#endif //NDEBUG
handle->object->in_use--;
LEAVE_CS;
set_thread_id_record(handle->object, LOCK_STATE_NONE);
pthread_ret = pthread_rwlock_unlock(&handle->object->rwlock);
if (fist_on("com.pthread")) pthread_ret = FIST_PTHREAD_ERRCODE;
Expand All @@ -1305,7 +1317,6 @@
}

error_return:
LEAVE_CS;
if (ret != MTC_SUCCESS)
{
log_status(ret, NULL);
Expand Down
97 changes: 59 additions & 38 deletions daemon/sm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,16 +808,13 @@
}


MTC_STATIC void
rendezvous(
SM_PHASE phase1,
SM_PHASE phase2,
SM_PHASE phase3,
MTC_BOOLEAN on_heartbeat,
MTC_BOOLEAN on_statefile)
{
#if RENDEZVOUS_FAULT_HANDLING
void rendezvous_wait(SM_PHASE p1, SM_PHASE p2)
MTC_STATIC void
rendezvous_wait(
SM_PHASE p1,
SM_PHASE p2,
MTC_BOOLEAN on_heartbeat,
MTC_BOOLEAN on_statefile)
{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to de-indent the new rendezvous_wait() too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a separate commit that deindents these

PCOM_DATA_SM psm;
PCOM_DATA_HB phb;
Expand Down Expand Up @@ -898,12 +895,22 @@
hb_SF_cancel_accelerate();
}
}
#endif

MTC_STATIC void
rendezvous(
SM_PHASE phase1,
SM_PHASE phase2,
SM_PHASE phase3,
MTC_BOOLEAN on_heartbeat,
MTC_BOOLEAN on_statefile)
{
#if RENDEZVOUS_FAULT_HANDLING
set_sm_phase(phase1);
rendezvous_wait(phase1, phase2);
rendezvous_wait(phase1, phase2, on_heartbeat, on_statefile);

set_sm_phase(phase2);
rendezvous_wait(phase2, phase3);
rendezvous_wait(phase2, phase3, on_heartbeat, on_statefile);
#else
set_sm_phase(phase2);
#endif
Expand Down Expand Up @@ -1863,15 +1870,11 @@


MTC_STATIC MTC_BOOLEAN
update_sfdomain()
{
MTC_CLOCK now;
MTC_S8 hostmap[MAX_HOST_NUM + 1] = {0};
MTC_BOOLEAN changed = FALSE;

MTC_BOOLEAN update_sfdomain_sub(
update_sfdomain_sub(
MTC_CLOCK now,
MTC_S8 hostmap[MAX_HOST_NUM + 1],
MTC_BOOLEAN writable)
{
{
PCOM_DATA_SF psf;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, where you've de-indented the braces but not the rest of the function

MTC_BOOLEAN changed = FALSE;
MTC_S32 index;
Expand Down Expand Up @@ -1960,14 +1963,21 @@
}

return changed;
}
}

MTC_STATIC MTC_BOOLEAN
update_sfdomain()
{
MTC_CLOCK now;
MTC_S8 hostmap[MAX_HOST_NUM + 1] = {0};
MTC_BOOLEAN changed = FALSE;

now = _getms();

changed = update_sfdomain_sub(FALSE);
changed = update_sfdomain_sub(now, hostmap, FALSE);
if (changed)
{
changed = update_sfdomain_sub(TRUE);
changed = update_sfdomain_sub(now, hostmap, TRUE);
if (changed)
{
log_message(MTC_LOG_DEBUG,
Expand Down Expand Up @@ -2777,18 +2787,12 @@
}


MTC_STATIC MTC_BOOLEAN
sm_wait_signals_sm_hb_sf(
static MTC_BOOLEAN
check_sigs(
MTC_BOOLEAN sm_sig,
MTC_BOOLEAN hb_sig,
MTC_BOOLEAN sf_sig,
MTC_CLOCK timeout)
MTC_BOOLEAN sf_sig)
{
MTC_BOOLEAN signaled;
MTC_CLOCK start = _getms();

MTC_BOOLEAN check_sigs()
{
MTC_BOOLEAN signaled = FALSE;

if (sm_sig && smvar.sm_sig)
Expand All @@ -2808,27 +2812,44 @@
}

return signaled;
}
}

#define NSEC_PER_SEC 1000000000

MTC_STATIC MTC_BOOLEAN
sm_wait_signals_sm_hb_sf(
MTC_BOOLEAN sm_sig,
MTC_BOOLEAN hb_sig,
MTC_BOOLEAN sf_sig,
MTC_CLOCK timeout)
{
MTC_BOOLEAN signaled;

struct timespec deadline;

if (timeout == 0)
{
return FALSE;
}

struct timespec timeout_ts = mstots(timeout);
clock_gettime(CLOCK_REALTIME, &deadline);

Check warning

Code scanning / CodeChecker

the value returned by this function should not be disregarded; neglecting it may lead to errors Warning

the value returned by this function should not be disregarded; neglecting it may lead to errors
deadline.tv_nsec += timeout_ts.tv_nsec;
deadline.tv_sec += timeout_ts.tv_sec;
deadline.tv_sec += deadline.tv_nsec / NSEC_PER_SEC;
deadline.tv_nsec %= NSEC_PER_SEC;

pthread_mutex_lock(&smvar.mutex);
while (!(signaled = check_sigs()) &&
((timeout < 0)? TRUE: (_getms() - start < timeout)))
while (!(signaled = check_sigs(sm_sig, hb_sig, sf_sig)))
{
if (timeout < 0)
{
pthread_cond_wait(&smvar.cond, &smvar.mutex);
}
else
{
pthread_mutex_unlock(&smvar.mutex);
mssleep(100);
pthread_mutex_lock(&smvar.mutex);
}
pthread_cond_timedwait(&smvar.cond, &smvar.mutex, &deadline);
}
}
pthread_mutex_unlock(&smvar.mutex);

Expand Down
Loading