tp: fix dropped traces when merging same-remote-machine files (#6687)

Author: dreveman

src/trace_processor/importers/common/clock_tracker.cc

@@ -88,8 +88,31 @@ base::StatusOr<uint32_t> ClockTracker::AddSnapshot(
     }
     current_file_tag_ = own_file_id_;
   }
-  for (auto& ct : clock_timestamps)
+  // REALTIME is a single universal wall clock and the cross-machine rendezvous
+  // domain used to place files that share no other clock (see
+  // docs/concepts/merging-traces.md). A non-primary file on a *remote*
+  // (non-trace-time) machine reaches trace time only through that rendezvous,
+  // so relate each REALTIME clock it actually carries to the machine-canonical
+  // REALTIME at zero offset (a twin added to this snapshot). The trace-time
+  // machine is excluded: its events reach trace time directly through BOOTTIME.
+  const bool bridge_realtime =
+      !is_primary_ &&
+      machine_id_ != context_->trace_time_state->clock_id.machine_id;
+  std::vector<ClockTimestamp> canonical_realtime;
+  for (auto& ct : clock_timestamps) {
+    const uint32_t clock = ct.clock.id.clock_id;
     ct.clock.id = ClockId::Qualify(ct.clock.id, machine_id_, current_file_tag_);
+    if (PERFETTO_UNLIKELY(
+            bridge_realtime &&
+            (clock == protos::pbzero::BUILTIN_CLOCK_REALTIME ||
+             clock == protos::pbzero::BUILTIN_CLOCK_REALTIME_COARSE))) {
+      ClockTimestamp twin = ct;
+      twin.clock.id = ClockId::Qualify(ClockId::Machine(clock), machine_id_, 0);
+      canonical_realtime.push_back(twin);
+    }
+  }
+  clock_timestamps.insert(clock_timestamps.end(), canonical_realtime.begin(),
+                          canonical_realtime.end());
   return AddSnapshotInternal(clock_timestamps);
 }
 

src/trace_processor/importers/common/clock_tracker_unittest.cc

@@ -130,6 +130,43 @@ TEST_F(ClockTrackerTest, ClockDomainConversions) {
             static_cast<int64_t>(100000 - 1000 + 1e6));
 }
 
+// Merging independent traces from a remote machine deduplicates them onto one
+// machine: the first is the primary, the rest are non-primary. A non-primary
+// trace isolates its builtin clocks onto its own file tag so its snapshots
+// cannot corrupt the others' conversions. But a remote machine's only route to
+// trace time is the cross-machine REALTIME rendezvous (its BOOTTIME belongs to
+// a different machine), so a remote non-primary trace's REALTIME must stay on
+// the machine-canonical tag to join that rendezvous. Otherwise every one of its
+// events is dropped (clock_sync_failure_no_path).
+TEST_F(ClockTrackerTest, RemoteNonPrimaryFileResolvesThroughSharedRealtime) {
+  // Host machine (trace time == its BOOTTIME): relate REALTIME to BOOTTIME so
+  // the cross-machine REALTIME rendezvous can reach trace time.
+  ct_->AddSnapshot({{REALTIME, 1000}, {BOOTTIME, 1000}});
+
+  // Remote machine, primary trace: its own REALTIME<->BOOTTIME plus the
+  // deferred BOOTTIME->trace-time sync the remote reader registers. This builds
+  // the REALTIME(remote)<->REALTIME(host) rendezvous.
+  auto remote_primary = MakeRemoteTracker(/*raw_machine_id=*/1);
+  remote_primary->AddSnapshot({{REALTIME, 2000}, {BOOTTIME, 200000}});
+  remote_primary->AddDeferredClockSync(BOOTTIME);
+  ASSERT_TRUE(remote_primary->ToTraceTime(BOOTTIME, 200000).has_value());
+
+  // Remote machine, non-primary trace: a distinct file id but the SAME machine
+  // as remote_primary (reuse its machine_tracker; a real merge dedups files
+  // with the same raw machine id onto one machine row). Only its own
+  // REALTIME<->BOOTTIME. Its REALTIME must join the shared machine-canonical
+  // REALTIME to reach trace time via the rendezvous.
+  context_.trace_state =
+      TraceProcessorContextPtr<TraceProcessorContext::TraceState>::MakeRoot(
+          TraceProcessorContext::TraceState{TraceId(9)});
+  ClockTracker remote_np(&context_, primary_sync_.get(), /*is_primary=*/false);
+  remote_np.AddSnapshot({{REALTIME, 3000}, {BOOTTIME, 300000}});
+
+  // A BOOTTIME event on the non-primary trace reaches trace time only because
+  // its REALTIME is the shared machine-canonical node feeding the rendezvous.
+  EXPECT_TRUE(remote_np.ToTraceTime(BOOTTIME, 300000).has_value());
+}
+
 // When a clock moves backwards conversions *from* that clock are forbidden
 // but conversions *to* that clock should still work.
 // Think to the case of REALTIME going backwards from 3AM to 2AM during DST day.

src/trace_processor/importers/proto/proto_trace_reader.cc

@@ -1263,6 +1263,12 @@ base::Status ProtoTraceReader::OnPushDataToSorter() {
   for (auto& packet : eof_deferred_packets_) {
     RETURN_IF_ERROR(TimestampTokenizeAndPushToSorter(std::move(packet)));
   }
+  // Remote-machine readers are only ever reached by the dispatcher, never by
+  // the ForwardingTraceParser, so their own clock-deferred packets would
+  // otherwise never be flushed. Propagate EOF to them too.
+  for (auto it = machine_to_proto_readers_.GetIterator(); it; ++it) {
+    RETURN_IF_ERROR(it.value()->OnPushDataToSorter());
+  }
   return base::OkStatus();
 }