Skip to content

Conversation

@vasslitvinov
Copy link
Contributor

This PR fixes the shortcoming of this idiom in tests/optioned-server/auto-checkpoints.py:

assert directory_exists_delayed(autockptDir, 10)
delete_directory(autockptDir)

where the second line is executed after auto-checkpointing has started and therefore the directory has been created, and before auto-checkpointing is completed.

In such a case, removing the directory causes auto-checkpointing to fail. This goes unnoticed because the client has no way to know about it, however it throws off the carefully-carved sequence of events that the test code is intended to cause / observe, and testing fails.

How to detect when the server has finished creating a checkpoint, which we need before proceeding with next steps in testing? I added:
ak.client.wait_for_async_activity()
that makes the server wait for an in-progress auto-checkpointing, if any, to complete.

An alternative is for the server to create a marker file in the checkpoint directory upon the completion of a checkpoint. I did not implement this, however we may still want to add this. Another way is to "tail" arkouda.log to see if it ends with a line containing "finished autoCheckpoint".

Incidental changes:

  • Add currentTime() as a proposed replacement for Time.timeSinceEpoch().totalSeconds().
  • Prevent interruption of the server's "idle time" by a couple of messages like ruok and wait_for_async_activity.
  • Skip the processing of a message on the server if there is an error early in message handling.

@ShreyasKhandekar ShreyasKhandekar self-requested a review June 12, 2025 17:59
Copy link
Contributor

@ajpotts ajpotts left a comment

Choose a reason for hiding this comment

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

Looks good!

@ajpotts ajpotts added this pull request to the merge queue Jun 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 17, 2025
@ajpotts ajpotts added this pull request to the merge queue Jun 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 17, 2025
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
@vasslitvinov vasslitvinov force-pushed the fix-autockpt-testing branch from 8eb8d4a to a86da1a Compare June 17, 2025 17:39
@vasslitvinov
Copy link
Contributor Author

@ajpotts I just rebased my branch down to a single commit on top of the current master, while removing an unintended change to Logging.chpl - which motivated the rebase. I will keep an eye on the CI checks.

@ajpotts ajpotts added this pull request to the merge queue Jun 19, 2025
Merged via the queue into Bears-R-Us:master with commit 2920fbb Jun 19, 2025
27 checks passed
@vasslitvinov vasslitvinov deleted the fix-autockpt-testing branch June 24, 2025 18:18
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