Skip to content

Commit 0d742d0

Browse files
committed
DPL: pin per-input message identity in the relayer tests
The existing sections assert part counts and pointer nullness, never payload contents, so any change to how a slot's messages are stored can shuffle them between inputs without a single test noticing. Stamp every payload and check it comes back on the right input, in the order it was relayed. Two arrangements that a shared per-slot buffer makes interesting: arrivals interleaved across three inputs, so a cell is no longer the last one written when its second part shows up; and an expiring input materialised into a slot the other inputs already occupy, which leaves the cells out of input order. Contents are also re-checked after the slot has been refilled, which pins that consuming really does hand the messages over.
1 parent f16c6e4 commit 0d742d0

1 file changed

Lines changed: 205 additions & 0 deletions

File tree

Framework/Core/test/test_DataRelayer.cxx

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "Framework/ExpirationHandler.h"
3333
#include "Framework/LifetimeHelpers.h"
3434
#include <array>
35+
#include <cstring>
3536
#include <vector>
3637
#include <uv.h>
3738

@@ -968,4 +969,208 @@ TEST_CASE("DataRelayer")
968969
REQUIRE(activity2.expiredSlots == 0);
969970
REQUIRE(handlerCallCount == 1); // handler was not called a second time
970971
}
972+
973+
// Once the DataRelayer keeps a slot's messages in one shared buffer, every
974+
// input's parts live next to each other, so a slip in the offset bookkeeping
975+
// corrupts a *different* input's cell while leaving all the part counts
976+
// intact. Counting parts therefore cannot catch it: stamp each payload and
977+
// check identity. The arrival order below is interleaved on purpose -- after
978+
// step 2 input 0 is no longer the last cell, so step 3 has to relocate it,
979+
// and likewise input 1 at step 5.
980+
SECTION("InterleavedPartsKeepIdentity")
981+
{
982+
InputSpec spec0{"clusters", "TPC", "CLUSTERS"};
983+
InputSpec spec1{"its", "ITS", "CLUSTERS"};
984+
InputSpec spec2{"tracks", "TPC", "TRACKS"};
985+
986+
std::vector<InputRoute> inputs = {
987+
InputRoute{spec0, 0, "Fake0", 0},
988+
InputRoute{spec1, 1, "Fake1", 0},
989+
InputRoute{spec2, 2, "Fake2", 0},
990+
};
991+
992+
std::vector<InputChannelInfo> infos{1};
993+
TimesliceIndex index{1, infos};
994+
ref.registerService(ServiceRegistryHelpers::handleForService<TimesliceIndex>(&index));
995+
996+
auto policy = CompletionPolicyHelpers::consumeWhenAll();
997+
DataRelayer relayer(policy, inputs, index, {registry}, -1);
998+
relayer.setPipelineLength(1);
999+
1000+
auto transport = fair::mq::TransportFactory::CreateTransportFactory("zeromq");
1001+
auto channelAlloc = o2::pmr::getTransportAllocator(transport.get());
1002+
1003+
std::array<DataHeader, 3> prototypes;
1004+
prototypes[0].dataOrigin = "TPC";
1005+
prototypes[0].dataDescription = "CLUSTERS";
1006+
prototypes[1].dataOrigin = "ITS";
1007+
prototypes[1].dataDescription = "CLUSTERS";
1008+
prototypes[2].dataOrigin = "TPC";
1009+
prototypes[2].dataDescription = "TRACKS";
1010+
1011+
auto stampOf = [](size_t input, size_t part) -> uint32_t {
1012+
return 1000u * static_cast<uint32_t>(input + 1) + static_cast<uint32_t>(part);
1013+
};
1014+
1015+
auto relayOne = [&](size_t input, size_t part, size_t timeslice) {
1016+
DataHeader dh = prototypes[input];
1017+
dh.subSpecification = 0;
1018+
dh.splitPayloadIndex = 0;
1019+
dh.splitPayloadParts = 1;
1020+
dh.payloadSize = sizeof(uint32_t);
1021+
1022+
std::array<fair::mq::MessagePtr, 2> msgs;
1023+
msgs[0] = o2::pmr::getMessage(Stack{channelAlloc, dh, DataProcessingHeader{timeslice, 1}});
1024+
msgs[1] = transport->CreateMessage(sizeof(uint32_t));
1025+
uint32_t const stamp = stampOf(input, part);
1026+
memcpy(msgs[1]->GetData(), &stamp, sizeof(stamp));
1027+
DataRelayer::InputInfo info{0, 2, DataRelayer::InputType::Data, {ChannelIndex::INVALID}};
1028+
relayer.relay(msgs[0]->GetData(), msgs.data(), info, 2);
1029+
REQUIRE(msgs[0].get() == nullptr);
1030+
REQUIRE(msgs[1].get() == nullptr);
1031+
};
1032+
1033+
std::array<std::pair<size_t, size_t>, 5> const arrivals = {{{0, 0}, {1, 0}, {0, 1}, {2, 0}, {1, 1}}};
1034+
for (auto const& [input, part] : arrivals) {
1035+
relayOne(input, part, 0);
1036+
}
1037+
1038+
std::vector<RecordAction> ready;
1039+
relayer.getReadyToProcess(ready);
1040+
REQUIRE(ready.size() == 1);
1041+
REQUIRE(ready[0].op == CompletionPolicy::CompletionOp::Consume);
1042+
1043+
auto result = relayer.consumeAllInputsForTimeslice(ready[0].slot);
1044+
REQUIRE(result.size() == 3);
1045+
1046+
std::array<size_t, 3> const expectedParts = {2, 2, 1};
1047+
auto checkContents = [&]() {
1048+
for (size_t i = 0; i < 3; ++i) {
1049+
REQUIRE((result[i] | count_parts{}) == expectedParts[i]);
1050+
for (size_t p = 0; p < expectedParts[i]; ++p) {
1051+
auto& header = result[i] | get_header{p};
1052+
auto& payload = result[i] | get_payload{p, 0};
1053+
REQUIRE(header.get() != nullptr);
1054+
REQUIRE(payload.get() != nullptr);
1055+
uint32_t seen = 0;
1056+
memcpy(&seen, payload->GetData(), sizeof(seen));
1057+
REQUIRE(seen == stampOf(i, p));
1058+
}
1059+
}
1060+
};
1061+
checkContents();
1062+
1063+
// The consumed messages belong to the caller now. Refilling the very same
1064+
// slot must not disturb them, whether the relayer handed over vectors or an
1065+
// arena it has since reused.
1066+
relayOne(0, 0, 1);
1067+
checkContents();
1068+
}
1069+
1070+
// An expiring input is materialised straight into the slot, so with one
1071+
// shared buffer per slot it lands *after* whatever the other inputs already
1072+
// hold -- the cells are then no longer in input order. Check that the data
1073+
// which was already there survives the expiry untouched.
1074+
SECTION("ExpiryDoesNotDisturbNeighbours")
1075+
{
1076+
InputSpec dataSpec0{"clusters", "TPC", "CLUSTERS"};
1077+
InputSpec condSpec{"condition", "TST", "COND"};
1078+
InputSpec dataSpec2{"tracks", "TPC", "TRACKS"};
1079+
1080+
std::vector<InputRoute> inputs = {
1081+
InputRoute{dataSpec0, 0, "from_source_to_self", 0},
1082+
InputRoute{condSpec, 1, "from_source_to_self", 0},
1083+
InputRoute{dataSpec2, 2, "from_source_to_self", 0},
1084+
};
1085+
1086+
std::vector<InputChannelInfo> infos{1};
1087+
TimesliceIndex index{1, infos};
1088+
ref.registerService(ServiceRegistryHelpers::handleForService<TimesliceIndex>(&index));
1089+
1090+
FairMQDeviceProxy proxy;
1091+
std::vector<fair::mq::Channel> channels{fair::mq::Channel("from_source_to_self")};
1092+
auto findChannel = [&channels](std::string const& name) -> fair::mq::Channel& {
1093+
for (auto& ch : channels) {
1094+
if (ch.GetName() == name) {
1095+
return ch;
1096+
}
1097+
}
1098+
throw std::runtime_error("Channel not found: " + name);
1099+
};
1100+
proxy.bind({}, inputs, {}, findChannel, [] { return false; });
1101+
ref.registerService(ServiceRegistryHelpers::handleForService<FairMQDeviceProxy>(&proxy));
1102+
1103+
auto policy = CompletionPolicyHelpers::consumeWhenAll();
1104+
DataRelayer relayer(policy, inputs, index, {registry}, -1);
1105+
relayer.setPipelineLength(1);
1106+
1107+
auto transport = fair::mq::TransportFactory::CreateTransportFactory("zeromq");
1108+
auto channelAlloc = o2::pmr::getTransportAllocator(transport.get());
1109+
1110+
auto stampOf = [](size_t input) -> uint32_t { return 7000u + static_cast<uint32_t>(input); };
1111+
1112+
auto relayData = [&](size_t input, char const* origin, char const* description) {
1113+
DataHeader dh;
1114+
dh.dataOrigin.runtimeInit(origin);
1115+
dh.dataDescription.runtimeInit(description);
1116+
dh.subSpecification = 0;
1117+
dh.splitPayloadIndex = 0;
1118+
dh.splitPayloadParts = 1;
1119+
dh.payloadSize = sizeof(uint32_t);
1120+
std::array<fair::mq::MessagePtr, 2> msgs;
1121+
msgs[0] = o2::pmr::getMessage(Stack{channelAlloc, dh, DataProcessingHeader{0, 1}});
1122+
msgs[1] = transport->CreateMessage(sizeof(uint32_t));
1123+
uint32_t const stamp = stampOf(input);
1124+
memcpy(msgs[1]->GetData(), &stamp, sizeof(stamp));
1125+
DataRelayer::InputInfo info{0, 2, DataRelayer::InputType::Data, {ChannelIndex::INVALID}};
1126+
relayer.relay(msgs[0]->GetData(), msgs.data(), info, 2);
1127+
REQUIRE(msgs[0].get() == nullptr);
1128+
};
1129+
1130+
// The two data inputs arrive first, so the slot is already occupied when
1131+
// the condition expires into it.
1132+
relayData(0, "TPC", "CLUSTERS");
1133+
relayData(2, "TPC", "TRACKS");
1134+
1135+
DataHeader condDh{"COND", "TST", 0};
1136+
condDh.splitPayloadParts = 1;
1137+
condDh.splitPayloadIndex = 0;
1138+
DataProcessingHeader condDph{0, 1};
1139+
1140+
ExpirationHandler handler;
1141+
handler.name = "test-condition";
1142+
handler.routeIndex = RouteIndex{1};
1143+
handler.lifetime = Lifetime::Condition;
1144+
// Deliberately *not* a fresh slot: return the one the data is already in,
1145+
// which is what puts the materialised cell out of input order.
1146+
handler.creator = [](ServiceRegistryRef, ChannelIndex) -> TimesliceSlot {
1147+
return TimesliceSlot{0};
1148+
};
1149+
handler.checker = LifetimeHelpers::expireAlways();
1150+
handler.handler = [&transport, &channelAlloc, &condDh, &condDph](ServiceRegistryRef, PartRef& part, data_matcher::VariableContext&) {
1151+
part.header = o2::pmr::getMessage(o2::header::Stack{channelAlloc, condDh, condDph});
1152+
part.payload = transport->CreateMessage(4);
1153+
};
1154+
1155+
std::vector<ExpirationHandler> handlers{handler};
1156+
auto activity = relayer.processDanglingInputs(handlers, {registry}, true);
1157+
REQUIRE(activity.expiredSlots == 1);
1158+
1159+
std::vector<RecordAction> ready;
1160+
relayer.getReadyToProcess(ready);
1161+
REQUIRE(ready.size() == 1);
1162+
REQUIRE(ready[0].op == CompletionPolicy::CompletionOp::Consume);
1163+
1164+
auto result = relayer.consumeAllInputsForTimeslice(ready[0].slot);
1165+
REQUIRE(result.size() == 3);
1166+
REQUIRE((result[1] | count_parts{}) == 1);
1167+
for (size_t i : {0u, 2u}) {
1168+
REQUIRE((result[i] | count_parts{}) == 1);
1169+
auto& payload = result[i] | get_payload{0, 0};
1170+
REQUIRE(payload.get() != nullptr);
1171+
uint32_t seen = 0;
1172+
memcpy(&seen, payload->GetData(), sizeof(seen));
1173+
REQUIRE(seen == stampOf(i));
1174+
}
1175+
}
9711176
}

0 commit comments

Comments
 (0)