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.

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

[#343] Ensure Ctrl-C on a long-running ZWRITE clears the internal zwrite-is-active flag or else debug-only assert failures can occur #344

Merged
merged 1 commit into from
Aug 22, 2018

Conversation

nars1
Copy link
Collaborator

@nars1 nars1 commented Aug 15, 2018

TREF(in_zwrite) is set to TRUE at the start of a ZWRITE. If the ZWRITE is run in direct mode (i.e. the
YDB> prompt) and runs for a long time so the user interrupts it with a Ctrl-C, a YDB-I-CTLRC error is
issued and control returns to the YDB> prompt. After that if the user runs an M command that contains
an lvn, one gets an assert failure while running a debug build of YottaDB.

%YDB-F-ASSERT, Assert failed in sr_port/gtm_fetch.c line 73 for expression (!TREF(in_zwrite))

This is because TREF(in_zwrite) was not set to FALSE when the Ctrl-C happened. It should have been reset
in mdb_condition_handler() but that had code to invoke NULLIFY_MERGE_ZWRITE_CONTEXT (the macro which
resets TREF(in_zwrite) among other things) only if the error severity is ERROR or FATAL. But the CTRLC
error severity is INFO and so it did not clear TREF(in_zwrite).

In V6.2-001, mdb_condition_handler() did the clear of TREF(in_zwrite) unconditionally but in V6.2-002,
this was moved into a pre-existing if block (that checked whether severity is ERROR or FATAL) and folded
into a new macro NULLIFY_MERGE_ZWRITE_CONTEXT. It is not clear why this change was necessary. Clearing
the interrupted zwrite and/or merge context seems safe to do on any error even if it is a SUCCESS or
INFO type. So have moved the NULLIFY_MERGE_ZWRITE_CONTEXT macro outside the "if" block. This way all
zwrite-related context (including lvzwrite_block which is local-variable related context of ZWRITE) is
now reset on a CTRLC error. Seems the right thing to do.

…nal zwrite-is-active flag or else dbg-only assert failures can occur

TREF(in_zwrite) is set to TRUE at the start of a ZWRITE. If the ZWRITE is run in direct mode (i.e. the
YDB> prompt) and runs for a long time so the user interrupts it with a Ctrl-C, a YDB-I-CTLRC error is
issued and control returns to the YDB> prompt. After that if the user runs an M command that contains
an lvn, one gets an assert failure while running a debug build of YottaDB.

%YDB-F-ASSERT, Assert failed in sr_port/gtm_fetch.c line 73 for expression (!TREF(in_zwrite))

This is because TREF(in_zwrite) was not set to FALSE when the Ctrl-C happened. It should have been reset
in mdb_condition_handler() but that had code to invoke NULLIFY_MERGE_ZWRITE_CONTEXT (the macro which
resets TREF(in_zwrite) among other things) only if the error severity is ERROR or FATAL. But the CTRLC
error severity is INFO and so it did not clear TREF(in_zwrite).

In V6.2-001, mdb_condition_handler() did the clear of TREF(in_zwrite) unconditionally but in V6.2-002,
this was moved into a pre-existing if block (that checked whether severity is ERROR or FATAL) and folded
into a new macro NULLIFY_MERGE_ZWRITE_CONTEXT. It is not clear why this change was necessary. Clearing
the interrupted zwrite and/or merge context seems safe to do on any error even if it is a SUCCESS or
INFO type. So have moved the NULLIFY_MERGE_ZWRITE_CONTEXT macro outside the "if" block. This way all
zwrite-related context (including lvzwrite_block which is local-variable related context of ZWRITE) is
now reset on a CTRLC error. Seems the right thing to do.
@nars1 nars1 self-assigned this Aug 15, 2018
@nars1 nars1 requested a review from estess August 15, 2018 19:49
@chathaway-codes chathaway-codes merged commit c719bc3 into YottaDB:master Aug 22, 2018
chathaway-codes pushed a commit that referenced this pull request Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants