Skip to content

Remove tracker#3446

Merged
robgjansen merged 3 commits intoshadow:mainfrom
robgjansen:remove-tracker
Dec 1, 2024
Merged

Remove tracker#3446
robgjansen merged 3 commits intoshadow:mainfrom
robgjansen:remove-tracker

Conversation

@robgjansen
Copy link
Copy Markdown
Member

@robgjansen robgjansen commented Nov 23, 2024

The tracker is C code that is blocking converting other modules such as DNS to rust cleanly. We previously decided that we would no longer support the C tracker code, and that new statistics should be handled in rust and logged to data files rather than the log file.

Closes #800, #1399

Progress on #1450, #1719

Stats planning: #2727

@robgjansen robgjansen added Component: Main Composing the core Shadow executable Component: Tools Peripheral tools like parsing log files or visualizing results Component: Documentation In-repository documentation, under docs/ labels Nov 23, 2024
@robgjansen robgjansen self-assigned this Nov 23, 2024
@github-actions github-actions bot added the Component: Testing Unit and integration tests and frameworks label Nov 23, 2024
@robgjansen robgjansen marked this pull request as ready for review November 23, 2024 21:28
@robgjansen
Copy link
Copy Markdown
Member Author

I don't think this requires a version bump because tracker output was put into the experimental section and thus we can change it at any time.

@robgjansen
Copy link
Copy Markdown
Member Author

Not sure if dropping the tracker is controversial or not; happy to have @sporksmith weigh in too.

AFAIK, the log lines and information that are dropped are not used in any of the tornettools parsing. Tornettools does call the tgen plotting scripts, and it has some of its own plotting, but I don't think it uses the tracker info.

HOWEVER, I just realized that tracker output is currently being parsed and plotted in our nightly/weekly benchmark processes. For example:

The above produces graphs like this:
https://github.com/shadow/benchmark-results/blob/master/tgen/2024-11-27-T03-28-15/plots/shadow.results.pdf

I think we're just using these tracker-related scripts because they existed, but I don't think they are particularly helpful, especially since we already have other tgen or tornettools plots that are more helpful. So I don't think the fact that we are running them in the benchmarks is a good enough argument to prevent further progress on c migration.

@sporksmith
Copy link
Copy Markdown
Contributor

The above produces graphs like this:
https://github.com/shadow/benchmark-results/blob/master/tgen/2024-11-27-T03-28-15/plots/shadow.results.pdf

Are the other files in that directory all coming from tornettools plot? I don't see RAM usage or tornettool's combined pdf of all of its plots, but maybe they can be added in?

@robgjansen
Copy link
Copy Markdown
Member Author

robgjansen commented Nov 29, 2024

Are the other files in that directory all coming from tornettools plot? I don't see RAM usage or tornettool's combined pdf of all of its plots, but maybe they can be added in?

I linked to an example from the tgen nightly, and the other plots in that directory are coming from running tgentools on simulation tgen output. The graphs you mentioned are missing because we don't run tornettools on tgen-only simulations. The tornettools graphs do exist on the tor nightly though:

https://github.com/shadow/benchmark-results/tree/master/tor/2024-11-27-T04-14-19/plots

In the above output, I think the only file that we would need to remove if we merge this PR is shadow.results.pdf.

Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment

Choose a reason for hiding this comment

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

Removing the tracker sounds good to me! I agree that we'll need to remove the shadow.results.pdf and everything that generates it from the benchmark repo. That change will probably need to be done first so that the benchmark CI doesn't break.

This should probably also have a changelog entry even though it's experimental.

@robgjansen
Copy link
Copy Markdown
Member Author

Removed tracker data from benchmarks
https://github.com/shadow/benchmark/commit/cb46d0a4abd2615ea58facbc47a481512f09697d

The tools were used to parse and plot Shadow log files, including
information logged by the now-removed tracker and other log file
information (e.g., rusage() output). The tracker is removed, and
in general Shadow no longer supports users parsing information
from the log files, so the parsing tools are being removed too.
@robgjansen robgjansen merged commit 62b9a88 into shadow:main Dec 1, 2024
@robgjansen robgjansen deleted the remove-tracker branch December 1, 2024 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Documentation In-repository documentation, under docs/ Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks Component: Tools Peripheral tools like parsing log files or visualizing results

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segementation fault when logging ram usage

3 participants