ctf: Fix unreliable handling of invalid location
This could lead to bad seeks when opening experiments (bug 476816).
In experiments, it is often the case (expected) that a seek to a
location will result to an invalid location for one of the traces in
the experiment.
There are issues with the handling of seeking to an invalid
location (CtfLocation.INVALID_LOCATION)
1. In one spot, == is used instead of .equals which means that a
location read from the index is not considered invalid, but rather,
it seeks to an offset of 0.
2. Even if the location is properly considered invalid (== fixed),
the code relies on the "last packet end time" to seek past the end of
the trace which then make the context location become invalid. This
works correctly when the trace was fully read the first time but the
"last packet end time" is not necessarily the end of the trace when
it is opened a second time.
Instead, we can explicitly check for CtfLocation.INVALID_LOCATION
in CtfTmfContext.setLocation and CtfIterator.seek to make sure no
bad seeks occur.
Bug: 476816
Change-Id: I4c74259ae3b67fb22cae302f2d5f09f1adda51f0
Signed-off-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/55429
Reviewed-by: Hudson CI
Reviewed-on: https://git.eclipse.org/r/55700
Reviewed-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
Tested-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/AllTests.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/AllTests.java
index bada9f6..ab4eabe 100644
--- a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/AllTests.java
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/AllTests.java
@@ -29,6 +29,7 @@
org.eclipse.tracecompass.tmf.ctf.core.tests.event.AllTests.class,
org.eclipse.tracecompass.tmf.ctf.core.tests.iterator.AllTests.class,
org.eclipse.tracecompass.tmf.ctf.core.tests.trace.AllTests.class,
+ org.eclipse.tracecompass.tmf.ctf.core.tests.trace.indexer.AllTests.class,
/* Tests in other packages (that are there because of CTF) */
org.eclipse.tracecompass.tmf.ctf.core.tests.temp.request.AllTests.class,
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/CtfTmfTraceTest.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/CtfTmfTraceTest.java
index 87600ba..90f46e6 100644
--- a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/CtfTmfTraceTest.java
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/CtfTmfTraceTest.java
@@ -381,6 +381,26 @@
}
/**
+ * Run the ITmfContext seekEvent(ITmfLocation<?>) method test with invalid location.
+ */
+ @Test
+ public void testSeekEventInvalidLocation() {
+ CtfLocation ctfLocation = new CtfLocation(CtfLocation.INVALID_LOCATION);
+ ITmfContext result = fixture.seekEvent(ctfLocation);
+ assertNull(fixture.getNext(result));
+ assertEquals(CtfLocation.INVALID_LOCATION, result.getLocation().getLocationInfo());
+ result.dispose();
+
+ // Not using CtfLocation.INVALID_LOCATION directly on purpose, to make sure CtfLocationInfo.equals is properly used
+ CtfLocationInfo invalidLocation = new CtfLocationInfo(CtfLocation.INVALID_LOCATION.getTimestamp(), CtfLocation.INVALID_LOCATION.getIndex());
+ ctfLocation = new CtfLocation(invalidLocation);
+ result = fixture.seekEvent(ctfLocation);
+ assertNull(fixture.getNext(result));
+ assertEquals(CtfLocation.INVALID_LOCATION, result.getLocation().getLocationInfo());
+ result.dispose();
+ }
+
+ /**
* Run the boolean validate(IProject,String) method test.
*/
@Test
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/AllTests.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/AllTests.java
new file mode 100644
index 0000000..0743ae3
--- /dev/null
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/AllTests.java
@@ -0,0 +1,24 @@
+/*******************************************************************************
+ * Copyright (c) 2015 Ericsson
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v1.0 which
+ * accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *******************************************************************************/
+
+package org.eclipse.tracecompass.tmf.ctf.core.tests.trace.indexer;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+/**
+ * Test suite
+ */
+@RunWith(Suite.class)
+@Suite.SuiteClasses({
+ CtfExperimentCheckpointIndexTest.class,
+})
+public class AllTests {
+
+}
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/CtfExperimentCheckpointIndexTest.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/CtfExperimentCheckpointIndexTest.java
new file mode 100644
index 0000000..94f7220
--- /dev/null
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/CtfExperimentCheckpointIndexTest.java
@@ -0,0 +1,181 @@
+/*******************************************************************************
+ * Copyright (c) 2015 Ericsson
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v1.0 which
+ * accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *******************************************************************************/
+
+package org.eclipse.tracecompass.tmf.ctf.core.tests.trace.indexer;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+
+import java.io.File;
+
+import org.eclipse.tracecompass.ctf.core.tests.shared.CtfTestTrace;
+import org.eclipse.tracecompass.tmf.core.event.ITmfEvent;
+import org.eclipse.tracecompass.tmf.core.trace.ITmfContext;
+import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace;
+import org.eclipse.tracecompass.tmf.core.trace.TmfContext;
+import org.eclipse.tracecompass.tmf.core.trace.TmfTraceManager;
+import org.eclipse.tracecompass.tmf.core.trace.experiment.TmfExperiment;
+import org.eclipse.tracecompass.tmf.core.trace.indexer.ITmfTraceIndexer;
+import org.eclipse.tracecompass.tmf.core.trace.indexer.TmfBTreeTraceIndexer;
+import org.eclipse.tracecompass.tmf.core.trace.indexer.checkpoint.ITmfCheckpoint;
+import org.eclipse.tracecompass.tmf.core.trace.indexer.checkpoint.ITmfCheckpointIndex;
+import org.eclipse.tracecompass.tmf.ctf.core.tests.shared.CtfTmfTestTrace;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Test suite for indexing CTF experiments.
+ */
+public class CtfExperimentCheckpointIndexTest {
+
+ private static final String EXPERIMENT = "MyExperiment";
+ private static final CtfTmfTestTrace TEST_TRACE1 = CtfTmfTestTrace.TRACE2;
+ private static final CtfTmfTestTrace TEST_TRACE2 = CtfTmfTestTrace.KERNEL_VM;
+ private static final int NB_EVENTS = CtfTestTrace.TRACE2.getNbEvents() + CtfTestTrace.KERNEL_VM.getNbEvents();
+
+ private static final long START_TIME = 1331668247314038062L;
+ private static final long END_TIME = 1363700770550261288L;
+ private static final int BLOCK_SIZE = 50000;
+ private static final int LAST_EVENT_RANK = NB_EVENTS - 1;
+ private static final int LAST_CHECKPOINT_RANK = LAST_EVENT_RANK / BLOCK_SIZE;
+ private static final int NB_CHECKPOINTS = LAST_CHECKPOINT_RANK + 1;
+
+ private static ITmfTrace[] fTestTraces;
+ private static TmfExperiment fExperiment;
+ private static TestIndexer fIndexer;
+
+ /**
+ * Setup the test class
+ */
+ @BeforeClass
+ public static void setUpClass() {
+ assumeTrue(TEST_TRACE1.exists());
+ assumeTrue(TEST_TRACE2.exists());
+ }
+
+ /**
+ * Setup the test
+ */
+ @Before
+ public void setUp() {
+ deleteSupplementaryFiles();
+ setUpTraces();
+ }
+
+ private static void setUpTraces() {
+ fTestTraces = new ITmfTrace[2];
+ fTestTraces[0] = TEST_TRACE1.getTrace();
+ fTestTraces[1] = TEST_TRACE2.getTrace();
+ fExperiment = new TmfExperiment(ITmfEvent.class, EXPERIMENT, fTestTraces, BLOCK_SIZE, null) {
+ @Override
+ protected ITmfTraceIndexer createIndexer(int interval) {
+ fIndexer = new TestIndexer(this, interval);
+ return fIndexer;
+ }
+ };
+ fExperiment.indexTrace(true);
+ }
+
+ /**
+ * Tear down the test
+ */
+ @After
+ public void tearDown() {
+ deleteSupplementaryFiles();
+ disposeTraces();
+ }
+
+ private static void deleteSupplementaryFiles() {
+ final String TRACE_DIRECTORY = TmfTraceManager.getTemporaryDirPath() + File.separator + EXPERIMENT;
+ File supplementaryFileDir = new File(TRACE_DIRECTORY);
+ if (supplementaryFileDir.exists()) {
+ for (File file : supplementaryFileDir.listFiles()) {
+ file.delete();
+ }
+ }
+ }
+
+ private static void disposeTraces() {
+ fExperiment.dispose();
+ fExperiment = null;
+ for (ITmfTrace trace : fTestTraces) {
+ trace.dispose();
+ }
+ fTestTraces = null;
+ }
+
+ /**
+ * Test indexer to give access to checkpoints
+ */
+ private static class TestIndexer extends TmfBTreeTraceIndexer {
+
+ public TestIndexer(ITmfTrace trace, int interval) {
+ super(trace, interval);
+ }
+
+ public ITmfCheckpointIndex getCheckpoints() {
+ return getTraceIndex();
+ }
+ }
+
+ /**
+ * Test the content of the index after building the full index
+ */
+ @Test
+ public void testIndexing() {
+ assertTrue(fIndexer.getCheckpoints().isCreatedFromScratch());
+ verifyIndexContent();
+ }
+
+ /**
+ * Test that a fully built index has the same content when reloaded from disk
+ */
+ @Test
+ public void testReopenIndex() {
+ assertTrue(fIndexer.getCheckpoints().isCreatedFromScratch());
+ disposeTraces();
+ setUpTraces();
+ assertFalse(fIndexer.getCheckpoints().isCreatedFromScratch());
+ verifyIndexContent();
+ }
+
+ private static void verifyIndexContent() {
+ assertEquals("getTraceSize", NB_EVENTS, fExperiment.getNbEvents());
+ assertEquals("getRange-start", START_TIME, fExperiment.getTimeRange().getStartTime().getValue());
+ assertEquals("getRange-end", END_TIME, fExperiment.getTimeRange().getEndTime().getValue());
+ assertEquals("getStartTime", START_TIME, fExperiment.getStartTime().getValue());
+ assertEquals("getEndTime", END_TIME, fExperiment.getEndTime().getValue());
+
+ ITmfCheckpointIndex checkpoints = fIndexer.getCheckpoints();
+ assertTrue(checkpoints != null);
+ assertEquals(NB_EVENTS, checkpoints.getNbEvents());
+ assertEquals(NB_CHECKPOINTS, checkpoints.size());
+
+ // Validate that each checkpoint points to the right event
+ for (int i = 0; i < checkpoints.size(); i++) {
+ ITmfCheckpoint checkpoint = checkpoints.get(i);
+ TmfContext context = new TmfContext(checkpoint.getLocation(), i * BLOCK_SIZE);
+ ITmfEvent event = fExperiment.parseEvent(context);
+ assertEquals(context.getRank(), i * BLOCK_SIZE);
+ assertEquals(0, (checkpoint.getTimestamp().compareTo(event.getTimestamp())));
+ }
+
+ ITmfContext context = fExperiment.seekEvent(0);
+ ITmfEvent event = fExperiment.getNext(context);
+ assertEquals(START_TIME, event.getTimestamp().getValue());
+
+ context = fExperiment.seekEvent(NB_EVENTS - 1);
+ event = fExperiment.getNext(context);
+ assertEquals(END_TIME, event.getTimestamp().getValue());
+ }
+}
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/internal/tmf/ctf/core/trace/iterator/CtfIterator.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/internal/tmf/ctf/core/trace/iterator/CtfIterator.java
index 42c5433..784ef49 100644
--- a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/internal/tmf/ctf/core/trace/iterator/CtfIterator.java
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/internal/tmf/ctf/core/trace/iterator/CtfIterator.java
@@ -178,6 +178,10 @@
*/
public synchronized boolean seek(CtfLocationInfo ctfLocationData) {
boolean ret = false;
+ if (ctfLocationData.equals(CtfLocation.INVALID_LOCATION)) {
+ fCurLocation = NULL_LOCATION;
+ return false;
+ }
/* Avoid the cost of seeking at the current location. */
if (fCurLocation.getLocationInfo().equals(ctfLocationData)) {
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/context/CtfTmfContext.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/context/CtfTmfContext.java
index be6b91e..b80b185 100644
--- a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/context/CtfTmfContext.java
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/context/CtfTmfContext.java
@@ -73,9 +73,14 @@
@Override
public synchronized void setLocation(ITmfLocation location) {
if (location instanceof CtfLocation) {
- CtfIterator iterator = getIterator();
- iterator.seek(((CtfLocation) location).getLocationInfo());
- fCurLocation = iterator.getLocation();
+ CtfLocation ctfLocation = (CtfLocation) location;
+ if (location.getLocationInfo().equals(CtfLocation.INVALID_LOCATION)) {
+ fCurLocation = ctfLocation;
+ } else {
+ CtfIterator iterator = getIterator();
+ iterator.seek(ctfLocation.getLocationInfo());
+ fCurLocation = iterator.getLocation();
+ }
} else {
fCurLocation = null;
}
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/trace/CtfTmfTrace.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/trace/CtfTmfTrace.java
index 1e06b71..87f4486 100644
--- a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/trace/CtfTmfTrace.java
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/trace/CtfTmfTrace.java
@@ -321,9 +321,6 @@
context.setRank(0);
} else {
context.setRank(ITmfContext.UNKNOWN_RANK);
- if (currentLocation.getLocationInfo() == CtfLocation.INVALID_LOCATION) {
- currentLocation = new CtfLocation(fTrace.getCurrentEndTime() + 1, 0L);
- }
}
/* This will seek and update the location after the seek */
context.setLocation(currentLocation);
diff --git a/tmf/org.eclipse.tracecompass.tmf.core.tests/src/org/eclipse/tracecompass/tmf/core/tests/trace/indexer/checkpoint/TmfExperimentCheckpointIndexTest.java b/tmf/org.eclipse.tracecompass.tmf.core.tests/src/org/eclipse/tracecompass/tmf/core/tests/trace/indexer/checkpoint/TmfExperimentCheckpointIndexTest.java
index a1aa4ca..ad18de4 100644
--- a/tmf/org.eclipse.tracecompass.tmf.core.tests/src/org/eclipse/tracecompass/tmf/core/tests/trace/indexer/checkpoint/TmfExperimentCheckpointIndexTest.java
+++ b/tmf/org.eclipse.tracecompass.tmf.core.tests/src/org/eclipse/tracecompass/tmf/core/tests/trace/indexer/checkpoint/TmfExperimentCheckpointIndexTest.java
@@ -56,6 +56,9 @@
private static final TmfTestTrace TEST_TRACE2 = TmfTestTrace.E_TEST_10K;
private static int NB_EVENTS = 20000;
private static int BLOCK_SIZE = 1000;
+ private static int LAST_EVENT_RANK = NB_EVENTS - 1;
+ private static int LAST_CHECKPOINT_RANK = LAST_EVENT_RANK / BLOCK_SIZE;
+ private static int NB_CHECKPOINTS = LAST_CHECKPOINT_RANK + 1;
private static ITmfTrace[] fTestTraces;
private static TmfExperimentStub fExperiment;
@@ -118,7 +121,7 @@
ITmfCheckpointIndex checkpoints = fExperiment.getIndexer().getCheckpoints();
int pageSize = fExperiment.getCacheSize();
assertTrue("Checkpoints exist", checkpoints != null);
- assertEquals("Checkpoints size", NB_EVENTS / BLOCK_SIZE, checkpoints.size());
+ assertEquals("Checkpoints size", NB_CHECKPOINTS, checkpoints.size());
// Validate that each checkpoint points to the right event
for (int i = 0; i < checkpoints.size(); i++) {
@@ -165,13 +168,13 @@
// Validate that each checkpoint points to the right event
ITmfCheckpointIndex checkpoints = experiment.getIndexer().getCheckpoints();
assertTrue("Checkpoints exist", checkpoints != null);
- assertEquals("Checkpoints size", NB_EVENTS / BLOCK_SIZE / 2, checkpoints.size());
+ assertEquals("Checkpoints size", NB_CHECKPOINTS / 2, checkpoints.size());
// Build the second half of the index
experiment.getIndexer().buildIndex(NB_EVENTS / 2, TmfTimeRange.ETERNITY, true);
// Validate that each checkpoint points to the right event
- assertEquals("Checkpoints size", NB_EVENTS / BLOCK_SIZE, checkpoints.size());
+ assertEquals("Checkpoints size", NB_CHECKPOINTS, checkpoints.size());
for (int i = 0; i < checkpoints.size(); i++) {
ITmfCheckpoint checkpoint = checkpoints.get(i);
ITmfLocation location = checkpoint.getLocation();
