ctf: Fix unreliable handling of invalid location
authorMarc-Andre Laperle <marc-andre.laperle@ericsson.com>
Mon, 7 Sep 2015 19:15:58 +0000 (15:15 -0400)
committerMarc-Andre Laperle <marc-andre.laperle@ericsson.com>
Thu, 10 Sep 2015 23:20:05 +0000 (19:20 -0400)
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
ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/AllTests.java
ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/CtfTmfTraceTest.java
ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/AllTests.java [new file with mode: 0644]
ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/trace/indexer/CtfExperimentCheckpointIndexTest.java [new file with mode: 0644]
ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/internal/tmf/ctf/core/trace/iterator/CtfIterator.java
ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/context/CtfTmfContext.java
ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/trace/CtfTmfTrace.java
tmf/org.eclipse.tracecompass.tmf.core.tests/src/org/eclipse/tracecompass/tmf/core/tests/trace/indexer/checkpoint/TmfExperimentCheckpointIndexTest.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/indexer/checkpoint/TmfCheckpointIndexer.java

index bada9f62beaa2830bda6b856fbc13c0943cd86ad..ab4eabeb2bcc532d685ffc0a1f9fef3fec3e327f 100644 (file)
@@ -29,6 +29,7 @@ import org.junit.runner.RunWith;
         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,
index efd6a214123448991e37e45a7ba59c662ff70c46..63fffffbd7e4aa6f1998127c8933e3c7480aef5d 100644 (file)
@@ -383,6 +383,26 @@ public class CtfTmfTraceTest {
         result.dispose();
     }
 
+    /**
+     * 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.
      */
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 (file)
index 0000000..0743ae3
--- /dev/null
@@ -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 (file)
index 0000000..94f7220
--- /dev/null
@@ -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());
+    }
+}
index 42c543397360d300b2368f8f78a9fb2c6dae2901..784ef49893b1e4188da8fb62322dc9660764cc3a 100644 (file)
@@ -178,6 +178,10 @@ public class CtfIterator extends CTFTraceReader
      */
     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)) {
index be6b91ed32115aa455a2d040b2a77d8a0c5dd89c..b80b185c2757b2be251679ec05a38785486bf4a4 100644 (file)
@@ -73,9 +73,14 @@ public class CtfTmfContext implements ITmfContext {
     @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;
         }
index 1c260df7d8bc0012f4cdc3ddfc4f1e8db7efb60b..bd44123a64a35b8acbbd63fe85082bba87741e76 100644 (file)
@@ -316,9 +316,6 @@ public class CtfTmfTrace extends TmfTrace
             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);
index a1aa4ca0265cd02cb9bc394832c8958d840951e2..ad18de40d9fa915f724f03dbe6efe8691b80b5ef 100644 (file)
@@ -56,6 +56,9 @@ public class TmfExperimentCheckpointIndexTest {
     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 @@ public class TmfExperimentCheckpointIndexTest {
         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 @@ public class TmfExperimentCheckpointIndexTest {
         // 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();
index 7e7f603524aac330812db5790beb3373af9e1964..43987b936b95bcf236c050060faff282a9ff4fd6 100644 (file)
@@ -314,19 +314,21 @@ public class TmfCheckpointIndexer implements ITmfTraceIndexer {
     /**
      * Position the trace at the given checkpoint
      *
-     * @param checkpoint the checkpoint index
+     * @param checkpointIndex the checkpoint index
      * @return the corresponding context
      */
-    private ITmfContext restoreCheckpoint(final long checkpoint) {
+    private ITmfContext restoreCheckpoint(final long checkpointIndex) {
         ITmfLocation location = null;
         long index = 0;
         synchronized (fTraceIndex) {
             if (!fTraceIndex.isEmpty()) {
-                index = checkpoint;
+                index = checkpointIndex;
                 if (index >= fTraceIndex.size()) {
                     index = fTraceIndex.size() - 1;
                 }
-                location = fTraceIndex.get(index).getLocation();
+                ITmfCheckpoint checkpoint = fTraceIndex.get(index);
+                TmfCoreTracer.traceIndexer("Restored checkpoint: " + checkpoint); //$NON-NLS-1$
+                location = checkpoint.getLocation();
             }
         }
         final ITmfContext context = fTrace.seekEvent(location);
This page took 0.045211 seconds and 5 git commands to generate.