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

Added feature to shutdown the logger properly if the server crashes unexpectedly #16

Merged
merged 3 commits into from
Sep 18, 2024
Merged

Added feature to shutdown the logger properly if the server crashes unexpectedly #16

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19df273
Added feature to shutdown the logger properly if the server crashes u…
PotatoPeeler3000 Sep 16, 2024
9bbd628
Another helpful print noting that things may have already shutdown pr…
PotatoPeeler3000 Sep 16, 2024
a05499d
Updated if statement and comment
PotatoPeeler3000 Sep 17, 2024
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
26 changes: 26 additions & 0 deletions src/main/java/us/ihmc/robotDataLogger/logger/YoVariableLoggerListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@

public class YoVariableLoggerListener implements YoVariablesUpdatedListener
{
/**
* We wait this long before shutting down the logger, this prevents logging forever in the case where the server didn't
* shut down properly.
*/
private static final int TICKS_WITHOUT_DATA_BEFORE_SHUTDOWN = 5000;

private static final int FLUSH_EVERY_N_PACKETS = 250;
public static final long STATUS_PACKET_RATE = Conversions.secondsToNanoseconds(5.0);
private static final long VIDEO_RECORDING_TIMEOUT = Conversions.secondsToNanoseconds(1.0);
Expand Down Expand Up @@ -81,6 +87,8 @@ public class YoVariableLoggerListener implements YoVariablesUpdatedListener
private long currentIndex = 0;

private long lastReceivedTimestamp = Long.MIN_VALUE;
private long ticksWithoutNewTimestamp = 0;
private boolean alreadyShutDown = false;

private final Announcement request;
private final Consumer<Announcement> doneListener;
Expand Down Expand Up @@ -433,6 +441,11 @@ public void disconnected()

doneListener.accept(request);
}

if (alreadyShutDown)
{
LogTools.info("This may have already shutdown properly due to the server losing power for an extended amount of time");
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessarily because of loss of power. I think you should just make this say This may have already shutdown properly because the connection to the YoVariableServer was lost

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair, went with: "This may have already shutdown because the connection to the server was lost"

}
}

private final ExecutorService executor = Executors.newCachedThreadPool();
Expand Down Expand Up @@ -563,13 +576,26 @@ public void receivedTimestampOnly(long timestamp)
{
synchronized (timestampUpdater)
{
// We haven't received any new timestamps, shutdown the logger gracefully
if (ticksWithoutNewTimestamp == TICKS_WITHOUT_DATA_BEFORE_SHUTDOWN)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make this if (!alreadyShutDown && ticksWithoutNewTimestamp >= TICKS_WITHOUT_DATA_BEFORE_SHUTDOWN)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ya good call

{
LogTools.warn("Whoa whoa whoa, haven't received new timestamps in a while, maybe the server crashed without proper shutdown, stopping the logger...");
disconnected();
alreadyShutDown = true;
}

if (timestamp > lastReceivedTimestamp) // Check if this a newer timestamp. UDP is out of order and the TCP packets also call this function
{
for (int i = 0; i < videoDataLoggers.size(); i++)
{
videoDataLoggers.get(i).timestampChanged(timestamp);
}
lastReceivedTimestamp = timestamp;
ticksWithoutNewTimestamp = 0;
}
else
{
ticksWithoutNewTimestamp++;
}
}
}
Expand Down