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

[#341] epoch_interval setting in the journal file needs to be honored in all cases #342

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

nars1
Copy link
Collaborator

@nars1 nars1 commented Aug 14, 2018

Code fixes for GTM-8708 in GT.M V6.3-002 wanted to prevent the update helper writer process from
unnecessarily making calls to grab_crit_immediate() in case the instance had no updates.
It did this by setting the next epoch time (jbp->next_epoch_time) to a huge value (MAXUINT4)
the moment a wcs_flu() call was done to write a regular epoch (JRE gvstat) where it noticed an
idle epoch (JRI gvstat) had already been written (because of inactivity since the last update).
This caused the jbp->next_epoch_time vs jgbl.gbl_jrec_time check in updhelper_writer() to
automatically skip invoking the grab_crit_immediate() during periods of idleness.

But this introduced the #341 regression. It is now possible for a regular epoch to not be written
in a timely fashion. This is because once jbp->next_epoch_time gets set to MAXUINT4 as part of
an update, if the next update on the same journal file happens say N seconds later (where N could
be as high as 5 seconds, the hardcoded inactivity time before an idle epoch kicks in), the call
to t_end/tp_tend for the next update finds jbp->next_epoch_time set to MAXUINT4 and resets it to
the current time (which is the time of the next update, not the time of the prior update) plus
the epoch_interval. This means the next_epoch_time is set to a value that is incorrect by at most
N seconds. That is, it is possible for two updates to be separated by as much as N + epoch_interval
seconds without having an intervening EPOCH in the journal file. This violates the definition of
the epoch_interval setting.

The fix for this is to undo all the changes done as part of GTM-8708. So jbp->next_epoch_time is
never set to MAXUINT4. Instead, updhelper_writer.c is enhanced to check if an EPOCH is the most
recent record in the journal buffer (jbp->post_epoch_freeaddr == jbp->rsrv_freeaddr) and if so
we skip checking jbp->next_epoch_time (and invoking grab_crit_immediate()) altogether since we know
there is nothing left to be flushed in the journal file. This achieves the objective of GTM-8708.
And for #341, jbp->next_epoch_time is updated in wcs_flu() at the time when we notice an epoch is
the most recent journal record in the journal file. This ensures we honor the epoch_interval setting.

…honored in all cases

Code fixes for GTM-8708 in GT.M V6.3-002 wanted to prevent the update helper writer process from
unnecessarily making calls to grab_crit_immediate() in case the instance had no updates.
It did this by setting the next epoch time (jbp->next_epoch_time) to a huge value (MAXUINT4)
the moment a wcs_flu() call was done to write a regular epoch (JRE gvstat) where it noticed an
idle epoch (JRI gvstat) had already been written (because of inactivity since the last update).
This caused the jbp->next_epoch_time vs jgbl.gbl_jrec_time check in updhelper_writer() to
automatically skip invoking the grab_crit_immediate() during periods of idleness.

But this introduced the YottaDB#341 regression. It is now possible for a regular epoch to not be written
in a timely fashion. This is because once jbp->next_epoch_time gets set to MAXUINT4 as part of
an update, if the next update on the same journal file happens say N seconds later (where N could
be as high as 5 seconds, the hardcoded inactivity time before an idle epoch kicks in), the call
to t_end/tp_tend for the next update finds jbp->next_epoch_time set to MAXUINT4 and resets it to
the current time (which is the time of the next update, not the time of the prior update) plus
the epoch_interval. This means the next_epoch_time is set to a value that is incorrect by at most
N seconds. That is, it is possible for two updates to be separated by as much as N + epoch_interval
seconds without having an intervening EPOCH in the journal file. This violates the definition of
the epoch_interval setting.

The fix for this is to undo all the changes done as part of GTM-8708. So jbp->next_epoch_time is
never set to MAXUINT4. Instead, updhelper_writer.c is enhanced to check if an EPOCH is the most
recent record in the journal buffer (jbp->post_epoch_freeaddr == jbp->rsrv_freeaddr) and if so
we skip checking jbp->next_epoch_time (and invoking grab_crit_immediate()) altogether since we know
there is nothing left to be flushed in the journal file. This achieves the objective of GTM-8708.
And for YottaDB#341, jbp->next_epoch_time is updated in wcs_flu() at the time when we notice an epoch is
the most recent journal record in the journal file. This ensures we honor the epoch_interval setting.
@nars1 nars1 self-assigned this Aug 14, 2018
@nars1 nars1 requested a review from estess August 14, 2018 19:46
@nars1 nars1 merged commit 7451179 into YottaDB:master Aug 14, 2018
@nars1 nars1 deleted the epoch branch August 14, 2018 21:53
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.

2 participants