@davidrohr this should give you the behaviour you requested. Can you please test it before I merge it?

OK, thanks, I will try it. Before we merge this, we should wait for #3552 and alisw/alidist#2262 to have proper buffer sizes requested, and propper error handling when the buffer size is exceeded.

@ironMann do you understand why StfSender dies?

@kft @davidrohr AliceO2Group/DataDistribution@a18de74 ?

is this not needed here anymore?

@ironMann : Yes, I assume so. AMD ROCm is still building with CC7 devtoolset 7. Has this workaround been removed, or why is it failing now?

No it's still there. I think this problem spread to other binaries. It's weird, because the boost program_options is done in all binaries... Looks like linking of StfBuilder fails this time.
Can you rerun the builder with DataDistribution/dev ?

I cannot do it easily, perhaps @ktf can. But I am quite sure it is the same problem. So I'd just merge the change in DataDistribution and then retry here.

I cannot do it easily because it would require doing so in some sort of alidist / gpu builder, which at the moment we do not have (that's why it trickled through the CI).

Anyways, I think it's unrelated to the PR and more to the 0.7.5 bump I did yesterday. @davidrohr can you confirm this behaves as expected for you?

Then each test would require alidist bump?
Can you instead build DataDistribution/dev in the O2-gpu builder?

Also, I suggest we do the change for 2ULL << 30 in a separate PR once FairMQ changes it's default, otherwise people might be puzzled that the help says one thing and then we do another.

@rbx any objection to change the default in FairMQ?

I can build it manually in the gpucibuilder container, but I'll have to build from scratch since I don't have a setup anymore and don't have much time now. I can do it over the weekend.

@ironMann the correct thing would be to have a CI for AliceO2Group/DataDistribution which builds any PR entering there with the current master of alidist. I can set it up, but not in the next few hours.

Anyways, I think it's unrelated to the PR and more to the 0.7.5 bump I did yesterday. @davidrohr can you confirm this behaves as expected for you?

Which 0.7.5 bump? Will test it asap.

Which 0.7.5 bump? Will test it asap.

alisw/alidist#2267

Which 0.7.5 bump? Will test it asap.

This already tried to build 0.7.5 of DataDistribution (in alidist). However the changes in between versions broke the gpu builder.

@ironMann the correct thing would be to have a CI for AliceO2Group/DataDistribution which builds any PR entering there with the current master of alidist. I can set it up, but not in the next few hours.

This would be cool. Let me know if you need anything from me.
But, also run the GPU builder for alidist PRs?

We anyway decided in WP3 to merge the builders for checkcode / gpu / and linux o2.
I'll take care of this as soon as AMD supports a proper compiler instead of devtoolset7.
They have been delaying this since March. Let's see...
Then we shouldn't need additional builder or extensions of the current GPU builder.

GPU builder for alidist PRs

yeah, probably we should... The problem is that we are juggling too many builds at the moment and I need to do some reshuffling in the CI code before I am confident it will not break everything. For DataDistribution it's easy, because it's a small package at the tip of the dependency chain...

I will let you know if I need anything, but I should have superpowers.

OK, since this is urgent as O2 GPU CI is broken generally, I started a local build in the container now. Will report back in 2 hours when it is hopefully done.

@ktf : Building locally, it failed with arrow (see attached log).
I could fix it by adding an explicit ARROW_ROOT to alidist/o2.sh.
a.gz

@ironMann : The below patch fixes the DataDistribution compilation in the GPU CI for me (as expected). Could you add it to DataDistribution and bump it in alidist?

diff --git a/src/StfBuilder/runStfBuilderDevice.cxx b/src/StfBuilder/runStfBuilderDevice.cxx
index 623e6fd..916267f 100644
--- a/src/StfBuilder/runStfBuilderDevice.cxx
+++ b/src/StfBuilder/runStfBuilderDevice.cxx
@@ -22,6 +22,7 @@
 #include <runFairMQDevice.h>
 
 namespace bpo = boost::program_options;
+template class std::basic_string<char, std::char_traits<char>, std::allocator<char> >; // Workaround for bug in CC7 devtoolset7
 
 void addCustomOptions(bpo::options_description& options)
 {

@rbx any objection to change the default in FairMQ?

No objection, will change it to 2ULL << 30.

@davidrohr , I added the same for each executable. alisw/alidist#2273

@ktf : It seems applying the option works, but e.g.

o2-its-reco-workflow --disable-root-output | o2-tpc-reco-workflow --tpc-digit-reader '--infile tpcdigits.root' --input-type digits --output-type clusters,tracks,disable-writer --ca-clusterer | o2-tpcits-match-workflow --disable-root-input --disable-root-output | o2-tof-reco-workflow --disable-root-input --disable-root-output | o2-tpc-scdcalib-interpolation-workflow --disable-root-input --shm-segment-size 10000000

fails with

[ERROR] error while setting up workflow: Error while parsing serialised workflow

Shouldn't I get at some point an error that the memory size was exceeded?

@ktf : I tried to reduce the shared memory size because I wanted to see the memory allocation failing, but instead I god the message that the workflow failed to initialize. Dump attached.
dump.gz
log.gz

ah, ok.... I think it's unrelated from this change. You get that message when something goes wrong with the merging of the workflow. I will try to see if I can create a better error message.

I suggest we nevertheless merge this, objections?

So this seems to work for me.
Can be merged after #3552 is merged, otherwise the TPC workflow fails at default settings.

Just one comment: If I set the shared segment size too small, the workflow fails to initialize. I get errors like [ERROR] Unable to pass configuration to children at startup. I assume this is because the shared memory is also used to pass the configuration? Perhaps it would be good to have an error indicating one runs out of memory instead of that generic error?

No, the configuration is actually passed via stdout / stdin between the parent process and the children. I suspect when the memory is too low some printout scrambles the json passed between the two. I need to check.

@ktf : The TPC change is done, you can merge this as soon as you want.

With this PR I processingFcn of some devices not called.
For instance, o2-its-reco-workflow runs correctly digits reader, clusterizer, cluster-writer but not the tracker. I see its init() and endOfStream() called but not run(). No error message is printed.

If I revert a5815c3, everything works.

I've rebuild everything with fresh alidist, does not help.

Problem (ubuntu18.04) can be reproduced by

o2-sim -m PIPE ITS -n 10 -g pythia8
o2-sim-digitizer-workflow
o2-its-reco-workflow

with a5815c3 reverted there are lot of messages like

[16454:its-cooked-tracker]:  - Vertexer initialisation completed in: 0.004946 ms
[16454:its-cooked-tracker]:  - Tracklet finding completed in: 0.00351 ms
[16454:its-cooked-tracker]:  - Adjacent tracklets validation completed in: 0.000945 ms

and o2trac_its.root with a tree is written. W/o reverting there are no log and the output file is empty (created in init() and closed in endOfStream(), the

@ktf any hint how to debug this?

I've some issues as well on macOS. I suspect it's a regression in FairMQ 1.4.18, but have no further clue, for now. I made #3680 to revert.

@ktf : I tried @shahor02 commands above and I can reproduce the issue he sees.
I went back to FairMQ 1.4.16, and also there I see the issue, so it is not a regression in 1.4.18.

Ok, I will have a look.

So, I can now confirm there were two problems:

• One is the one being discussed in https://alice.its.cern.ch/jira/browse/O2-1485. This was preventing me to reproduce the issue in a consistent manner. We therefore need Try to improve session name hashing FairRootGroup/FairMQ#276 or equivalent fix.
• The one which affects this PR seems to be that in the forwarding the payload of ITS/CLUSTERS is nullptr when using shared memory (this is in DataProcessingDevice.cxx:forwardInputs(...)) and the check for nullptr silently skips it. Notice that the input.header is correct.

I confirm that without transport=shmem, input.payload is non null.

@matthiasrichter does it ring any bell?

@ktf yes, ITSMFT clusterized at the moment sends an empty vector ITS/CLUSTERS, this is a legacy feature which will be suppressed once MFT will be ready to use COMPCLUSTERS

ok, but why does "empty vector" mean nullptr in the shmem case?

Actually I remember there was a similar issue for the TPC. When there was no data in a sector, the event was lost before, and this was fixed by @matthiasrichter

Yes, I remember. Let me find matthias fix. Maybe it was in a separate part of the code.

* One is the one being discussed in https://alice.its.cern.ch/jira/browse/O2-1485. This was preventing me to reproduce the issue in a consistent manner. We therefore need [FairRootGroup/FairMQ#276](https://github.com/FairRootGroup/FairMQ/pull/276) or equivalent fix.

Is that only relevant for mac or also for linux?

I think it's relevant for both. What might mitigate it on linux is that maybe boost::hash(<uid> + dpl_<pid>) has more entropy due to the fact <uid> and <pid> are larger (e.g. on lxplus or simply because systemd uses more <pid>s). This might actually explain some random failures we have, as pids wrap around. Anyways, the best solution would be to have sha256 or some other cryptographic hash.

Anyways, the best solution would be to have sha256 or some other cryptographic hash.

Fully agree, so what prevents us from using sha2?

Indeed, its a similar problem like I have fixed for the InputRecord in #3578, we should have looked for other places immediately.

Empty vector means zero-length message, and such messages have been handled differently in FairMQ. While ZMQ transport provides a valid pointer and message length zero, the SHM transport returns nullptr for zero length messages.

I also feel crypto hash would be the best. My only hesitation is requiring openssl just for that. Another option is to use crc32, it's included in boost and gives different output for small/similar inputs (just tested on mac and linux). While not as robust as SHA, it may suffice for this use case. It is also short enough that it doesn't need the truncation.

Regarding nullptr, our intention is to change zmq transport to also return nullptr for empty messages.

Hm, but what exactly is the problem with openssl? I mean, it should be available everywhere, and for just SHA2, there should be no strict version requirement. On the other hand, for SHA2, we could also just use an ad-hoc implementation. Should not be that much code.

https://github.com/martynafford/sha-2/blob/master/src/sha2.hpp actually...

@matthiasrichter There is one more place where the payload is checked for nullptr. It should not matter (because it's in the timers path and timers always have a payload) however I will remove it, also in light of @rbx comment about the API evolution.

FairMQ v1.4.19 is now available with the fix for the shmem alignment (via MemoryResource), the nullptr return value for empty messages and the improved hashing algorithm (SHA2) to generate the shared memory id.