-
Notifications
You must be signed in to change notification settings - Fork 27
tmf.ctf: Re-initialize start and endtime after transform is applied #324
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
tmf.ctf: Re-initialize start and endtime after transform is applied #324
Conversation
4ea39e3 to
b9df9a6
Compare
| CtfTmfContext ctx = (CtfTmfContext) seekEvent(0L); | ||
| CtfTmfEvent event = getNext(ctx); | ||
| /* Verify context is valid. It's only invalid trace if trace.init() hasn't been called) */ | ||
| if ((ctx != null) && (ctx.getLocation() != null)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is for the case that setTimestampTransform() is called before initTrace() and it avoids a NullPointerException. The test CtfTmfReadBoundsTest.testRapidBoundsWithTransform() for example call the transform method before init.
b9df9a6 to
cc4ca63
Compare
1168b16 to
7f174a5
Compare
MatthewKhouzam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works on my machine with a mega-offset
7f174a5 to
d8e6877
Compare
With this the start and endtime is correct when opening the experiment that has automatically applied timestamp transformation applied. fixes eclipse-tracecompass#323 [Fixed] Incorrect start/end time in ITmfTrace instances in experiment [Added] API in TmfTrace to reset indexer Signed-off-by: Bernd Hufmann <[email protected]>
d8e6877 to
304c763
Compare
PatrickTasse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the reported issue.
I have some hesitation though, because potentially any trace (not just CtfTmfTrace) should consider their start/end time obsolete as soon as a timestamp transform is applied. And it's obsolete regardless of whether or when the indexer needs to be reset.
I wonder if that could be generalized in TmfTrace with start/end=transformed first event timestamp. It seems maybe the specific implementation in CtfTmfTrace to initialize end time based on first packet end time has not much real benefit as it's only a temporary value that gets quickly updated...
But we can reconsider that later if it ever becomes an issue.
Acknowledged. Right now only CTF does automatic time offsetting during initialization of the experiment its traces. Other trace types don't do that. Manual offsetting is done outside the trace instances and controlled by the UI model (non-server case). It closes the trace and deletes supplementary folders and when the experiment is re-opened and re-initialized. The automatic offsetting in this PR is a different case. Ideally it should be managed by the classes that instantiates the traces. I think this PR is good compromise until we have need for a broader solution. |
What it does
tmf.ctf: Re-initialize start and endtime after transform is applied
With this the start and endtime is correct when opening the experiment that has automatically applied timestamp transformation applied.
fixes #323
How to test
Follow steps in #323 and verify that start and end times of both traces in the experiment are correct when listing the experiment from the python client.
Follow-ups
Update
TraceManagerService.instantiateTrace()to not start indexing of trace during experiment creation. The trace will be indexed when experiment is indexed.Review checklist