Conversation
ekapadi
added
Framework
|
TODO: |
There was a problem hiding this comment.
Pull request overview
This PR introduces an ADARA-packet replay utility for SNS instruments to enable deterministic debugging of live-data workflows. The implementation includes record/playback/summarize modes, Unix Domain Socket (UDS) support, and improved handling of duplicate DEVICE_DESC_TYPE packets.
Changes:
- New Python-based ADARA packet replay utility with recording, playback, and summarization capabilities
- Support for Unix Domain Sockets in SNSLiveEventDataListener
- Changed duplicate device descriptor handling from exception to warning
- Comprehensive test suite and documentation
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 43 comments.
Show a summary per file
| File | Description |
|---|---|
| instrument/Facilities.xml | Added SNAP_shim test instrument with UDS address |
| Framework/LiveData/src/SNSLiveEventDataListener.cpp | UDS support and duplicate packet warning instead of error |
| Framework/LiveData/src/ADARA/utils/packet_playback/*.py | Core packet player implementation |
| Framework/LiveData/test/Python/*.py | Comprehensive test suite for packet player |
| dev-docs/source/Testing/LiveData/LiveDataPacketPlayback.rst | Detailed user documentation |
| Framework/Properties/Mantid.properties.template | Configuration properties for SNSLiveEventDataListener |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Framework/LiveData/src/ADARA/utils/packet_playback/adara_player_conf.yml
Outdated
Show resolved
Hide resolved
Framework/LiveData/test/Python/scripts/adara_player_sessions.yml
Outdated
Show resolved
Hide resolved
Framework/LiveData/test/Python/scripts/adara_player_cull_events.py
Outdated
Show resolved
Hide resolved
Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py
Outdated
Show resolved
Hide resolved
Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py
Outdated
Show resolved
Hide resolved
Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py
Outdated
Show resolved
Hide resolved
Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py
Outdated
Show resolved
Hide resolved
Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py
Outdated
Show resolved
Hide resolved
ekapadi
force-pushed
the
EWM13769_live_data_playback
branch
from
92ddf4e to
edbf0a1
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds a full ADARA packet playback subsystem: new Python modules (packet_player.py, session_server.py, adara_player.py), a YAML runtime config, tests and CI CMake entries, documentation, and diagram assets. Introduces many unit tests for packet handling, playback, server, and utilities. Small C++ changes improve exception handling and configuration keys in live-data listeners, and tests add watchdog/teardown improvements. No public C++ APIs were changed. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Framework/LiveData/test/ISISHistoDataListenerTest.h (1)
1-31:⚠️ Potential issue | 🟠 MajorRemove global
using namespacedeclarations to comply with clang-tidy.clang-tidy reports warnings on lines 32-36:
using namespace Mantid; // Line 32 using namespace Mantid::API; // Line 33 using namespace Mantid::LiveData; // Line 34 namespace { // Line 36These violate
cppcoreguidelines-avoid-non-const-global-variables. Replace globalusing namespacewith qualified names or function-scope declarations.Framework/API/src/LiveListenerFactory.cpp (1)
85-110:⚠️ Potential issue | 🟡 MinorAddress clang-tidy finding:
g_logshould be const.The exception handling improvements at lines 85–110 look good. However, clang-tidy reports a violation on line 18:
Kernel::Logger g_log("LiveListenerFactory");The
g_logvariable is non-const and globally accessible, which violatescppcoreguidelines-avoid-non-const-global-variables. Make it const:const Kernel::Logger g_log("LiveListenerFactory");
🤖 Fix all issues with AI agents
In `@Framework/LiveData/src/ADARA/utils/packet_playback/adara_player_conf.yml`:
- Line 17: The YAML comment for the socket_timeout key contains an extraneous
trailing backtick; open the adara_player_conf.yml entry for socket_timeout and
remove the backtick from the inline comment so it reads e.g. "socket_timeout:
1.0 # affects all socket types" (leave the key and value unchanged, only fix the
comment).
In `@Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py`:
- Around line 978-1002: The exceptional-socket check uses the wrong identifier:
change the check from "if socket in exceptional" to "if dest in exceptional" so
you're testing the actual socket returned by select.select (readable, writable,
exceptional). Also fix the buffer byte-count inflation when re-queueing on write
stalls: when you do packet_buffer.appendleft(pkt) in the write-stall branch
(function/method using packet_buffer, current_buffer_bytes, pkt), do NOT add
pkt.size to current_buffer_bytes (remove the current_buffer_bytes += pkt.size
there) because the packet size was never decremented when the packet was popped;
alternatively ensure sizes are decremented at the pop point—either approach
removes the double-counting that breaks refill logic.
- Around line 473-488: ClientHelloPacket.start_time is currently set from raw
EPICS seconds without applying EPICS_EPOCH_OFFSET, causing a ~20-year epoch
mismatch with Packet.timestamp; change ClientHelloPacket.__init__ to add
EPICS_EPOCH_OFFSET seconds when building self._start_time (so it matches
Packet.timestamp's epoch handling) and update is_StartOfRun_packet to compare
against the converted epoch value (i.e., the "1 second" sentinel shifted by
EPICS_EPOCH_OFFSET) using the same np.datetime64/np.timedelta64 arithmetic so
both timestamp and start_time are semantically aligned.
In `@Framework/LiveData/src/ADARA/utils/packet_playback/session_server.py`:
- Around line 284-320: Add an atomic "claim" operation to replace the separate
read/increment: implement a new method (e.g. claim_session_number()) that
acquires self._session_number.get_lock(), reads current value, increments it,
releases the lock and returns the claimed (pre-increment) index; update callers
to use claim_session_number() rather than calling the session_number property
and then increment_session_number(). Also refactor session_path and
session_files to accept an explicit session index (e.g. session_path_for(index)
and session_files_for(index)) or compute from a passed-in index so they use the
claimed value rather than calling session_number themselves; keep existing
session_number/session_path/session_files as thin wrappers only if necessary for
non-forked code. Ensure session_files continues to use self._sessions/islice
with the claimed 1-based index and raise the same errors when out of range.
In `@Framework/LiveData/test/ISISHistoDataListenerTest.h`:
- Around line 62-79: Replace the unsynchronized raw-pointer access to m_dae in
the watchdog with an atomic pointer and add a condition variable to allow early
exit: introduce std::atomic<FakeISISHistoDAE*> m_daePtr and a
std::condition_variable m_watchdogCv plus matching std::mutex, in setUp start
m_watchdog to wait on m_watchdogCv for WATCHDOG_TIMEOUT then load m_daePtr (use
memory_order_acquire) and call cancel() only if non-null and isRunning(); after
every assignment to m_dae in the test file store the raw pointer into m_daePtr
with m_daePtr.store(m_dae.get(), std::memory_order_release); on successful test
completion notify the watchdog via m_watchdogCv.notify_one() so it can exit
early, and in tearDown ensure notify + join and reset both m_dae and m_daePtr.
In `@Framework/LiveData/test/Python/CMakeLists.txt`:
- Around line 22-35: The LOGS environment variable is being set with literal
single quotes ("LOGS='.'") which makes the value include quotes; update the
set_property call that appends PROPERTY ENVIRONMENT for LOGS in the foreach over
TEST_PY_FILES so it sets LOGS=. (Locate the foreach block using TEST_PY_FILES,
the derived _suitename/_test_name variables and the set_property call and remove
the surrounding single quotes from the LOGS value so the environment variable is
the plain dot.)
In `@Framework/LiveData/test/Python/scripts/adara_player_cull_events.py`:
- Around line 57-63: The help string passed to parser.add_argument for the
"--keep-fraction" option contains an EN DASH (U+2013) in the range "0.0–1.0";
update the help text in the parser.add_argument call (the "--keep-fraction" /
"-k" argument) to use an ASCII hyphen-minus ("0.0-1.0") so the displayed help
uses standard hyphens and avoids copy-paste issues.
In `@Framework/LiveData/test/Python/scripts/adara_player_StartLiveData.py`:
- Around line 57-75: The PRINT block in PROCESSING_SCRIPT assumes pulse_ts is
set and indexes pulse_ts[0] / pulse_ts[1], which will raise if pulse_ts is None;
modify the code around pulse_ts (where proton_charge and isinstance(...,
FloatTimeSeriesProperty) are handled) to guard access—check if pulse_ts is not
None before printing its indices (or print a fallback message like "no pulse
times" / skip the pulse-times line) so that accessing pulse_ts[0] and
pulse_ts[1] only occurs when pulse_ts is a valid tuple.
In `@Framework/LiveData/test/Python/test_adara_player_play.py`:
- Around line 391-413: The call to Player.stream_packets uses the wrong argument
types: change the call to pass an iterable of Path objects (e.g., files_iter or
a list/iterator of Path) as the second parameter and a boolean dry_run (or omit
it) as the third; update the invocation of player.stream_packets(mock_socket,
Path("/fake"), ["*.adara"]) to match the signature stream_packets(self, dest:
socket.socket, files: Iterable[Path], dry_run: bool = False) and follow the same
argument pattern used in the other calls at lines 250/291/300/348 so the second
arg is an Iterable[Path] and the third is a bool.
In `@Framework/LiveData/test/Python/test_adara_player.py`:
- Around line 254-343: The test test_iter_files_multi_file_ordering is flaky
because time.sleep(0.001) doesn't guarantee distinct mtimes; replace those
sleeps by explicitly setting file modification times with os.utime on file3,
file2, and file1 to deterministic 1-second-spaced mtimes so
Player.iter_files(packet_ordering_scheme="mtime") yields files in the intended
order; update the three sleep locations to call os.utime(...) on the
corresponding Path objects (file3, file2, file1) with increasing epoch seconds
(e.g., base, base+1, base+2) and ensure os is imported.
🧹 Nitpick comments (8)
docs/source/concepts/PropertiesFile.rst (1)
262-275: Documentation entries are clear and well-structured.All four new property entries are properly documented with accurate descriptions and appropriate example values. The properties align with the functionality being added in this PR.
Optional enhancement for
SNSLiveEventDataListener.testAddressdescription:The PR objectives mention that SNSLiveEventDataListener was modified to accept Unix Domain Socket (UDS) addresses. While the current description and example are correct for TCP addresses, consider mentioning that UDS paths are also supported.
📝 Optional description enhancement
-| ``SNSLiveEventDataListener.testAddress`` | Fallback instrument server url used by the | ``127.0.0.1:12345`` | -| | SNS live-data listener. | | +| ``SNSLiveEventDataListener.testAddress`` | Fallback instrument server address used by the | ``127.0.0.1:12345`` or | +| | SNS live-data listener. Supports TCP addresses | ``/tmp/adara.sock`` | +| | (host:port) and Unix Domain Socket paths. | |Testing/Data/SystemTest/adara_player_input_data.tar.md5 (1)
1-1: Consider using standard MD5 checksum file format.The standard format for MD5 checksum files (compatible with
md5sum -c) includes the filename:ed241b996934a11a4358aa58f0473e02 adara_player_input_data.tarThis would allow direct verification using
md5sum -c adara_player_input_data.tar.md5and makes the file more self-documenting.📝 Standard format example
-ed241b996934a11a4358aa58f0473e02 +ed241b996934a11a4358aa58f0473e02 adara_player_input_data.tarNote: Use two spaces between the hash and filename, as per the
md5sumtool convention.Framework/LiveData/src/SNSLiveEventDataListener.cpp (1)
143-173: Consider logging exception details for testAddress connects.The testAddress path uses a catch-all and loses the exception detail; mirroring the explicit-address branch will improve diagnosability.
♻️ Suggested refinement
- try { - m_socket.connect(testAddress); // BLOCKING connect - } catch (...) { - g_log.error() << "Connection to " << testAddress.toString() << " failed.\n"; - return false; - } + try { + m_socket.connect(testAddress); // BLOCKING connect + } catch (const Poco::Exception &e) { + g_log.error() << "POCO Exception in connect(): " << e.name() << ": " << e.what() + << " code: " << e.code() << " type: " << typeid(e).name(); + return false; + } catch (const std::exception &e) { + g_log.error() << "STD Exception in connect(): " << e.what() + << " type: " << typeid(e).name(); + return false; + } catch (...) { + g_log.error() << "Unknown exception in connect(): " + << " type: " << typeid(*this).name(); + return false; + }Framework/LiveData/test/Python/test_adara_player_server.py (1)
333-336: Consider using_prefix for intentionally unused lambda parameter.The
patternsparameter is unused because the mock returns a fixed value regardless of input. Using_prefix makes the intent clearer and silences the linter warning.Suggested fix
mock.patch( "session_server.UnixGlob.multi_glob", - side_effect=lambda sp, patterns: iter([sp / "dummy.adara"]), + side_effect=lambda sp, _patterns: iter([sp / "dummy.adara"]), ) as mock_multi,Framework/LiveData/test/Python/test_adara_player_summarize.py (1)
15-23: Unused timeout handling code.
TimeoutExceptionandtimeout_handlerare defined but never used in this test file. Unliketest_adara_player_play.pyandtest_adara_player_record.py, this file doesn't usesignal.SIGALRMfor timeout detection.Consider removing unused code
-class TimeoutException(Exception): - """Raised when a test times out.""" - - pass - - -def timeout_handler(signum, frame): - """Signal handler used to detect potential infinite loops in tests.""" - raise TimeoutException("Test timed out - likely infinite loop") - - class Test_Player_summarize(unittest.TestCase):Framework/LiveData/test/Python/scripts/adara_player_cull_events.py (2)
1-2: Remove unusednoqadirective.The
# ruff: noqa: E402directive is flagged as unused because E402 isn't enabled in the ruff configuration.Suggested fix
#!/usr/bin/env python3 -# ruff: noqa: E402 # Module level import not at top of file """ Shrink ADARA packet files by randomly down-sampling event packets.
89-105: Using internal_summarizemethod creates implementation coupling.The script uses
player._summarize()which is a private implementation detail (prefixed with_). If the Player API changes, this script could break. Consider whether a public API should be exposed for this use case, or document this as an internal tool.Framework/LiveData/test/Python/test_adara_player.py (1)
429-431: Make zip strictness explicit to satisfy B905 and catch drift.🔧 Suggested tweak
- for i, expected in zip([1, 2, 3], expected_names): + for i, expected in zip([1, 2, 3], expected_names, strict=True):
Framework/LiveData/src/ADARA/utils/packet_playback/adara_player_conf.yml
Outdated
Show resolved
Hide resolved
Framework/PythonInterface/mantid/LiveData/ADARA/utils/packet_playback/packet_player.py
Show resolved
Hide resolved
Framework/LiveData/src/ADARA/utils/packet_playback/session_server.py
Outdated
Show resolved
Hide resolved
...ework/PythonInterface/test/python/mantid/LiveData/ADARA/utils/packet_playback/CMakeLists.txt
Show resolved
Hide resolved
.../test/python/mantid/LiveData/ADARA/utils/packet_playback/scripts/adara_player_cull_events.py
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py`:
- Around line 974-1004: stream_packets currently never applies
self._packet_filter so ignored packets are still processed; after popping pkt
from packet_buffer in the send loop (the block that sets pkt =
packet_buffer.popleft() and updates current_buffer_bytes), call
self._packet_filter(pkt) and if it indicates the packet should be ignored, skip
sending by continuing the loop (do not attempt select/send), leaving
current_buffer_bytes already adjusted; ensure this check runs before the
dry_run/select/send logic so ignore_packets takes effect for both dry_run and
normal sends.
In `@Framework/LiveData/src/SNSLiveEventDataListener.cpp`:
- Around line 162-174: The catch blocks in SNSLiveEventDataListener::connect()
misuse typeid in the catch(...) (typeid(*this).name() reports the listener, not
an exception) and the g_log.error() messages in the Poco and std::exception
handlers lack trailing newlines; update the Poco and std::exception logging
calls (in the catch(const Poco::Exception &e) and catch(const std::exception &e)
blocks) to append a newline ("\n") to the message, and change the catch(...)
block to log a simple "Unknown exception in connect()\n" (remove the
typeid(*this).name() usage) so the message is correct and future log lines are
not concatenated.
- Around line 98-101: Update the outdated NOTE comment about the configuration
property: remove or rewrite the comment that claims
"SNSLiveEventDataListener.keepPausedEvents" is undocumented; instead state that
the property is documented and reference the ConfigService usage
(ConfigService::Instance().getValue<bool>("SNSLiveEventDataListener.keepPausedEvents"))
and the member m_keepPausedEvents initialization
(keepPausedEvents.value_or(false)). Ensure the new comment no longer warns about
an undocumented key or about a now-nonexistent legacy key name, and keep the
comment concise and accurate about current behavior.
In `@Framework/LiveData/test/ISISHistoDataListenerTest.h`:
- Around line 25-32: Remove the unused include directive <algorithm> from
ISISHistoDataListenerTest.h: locate the include block in the header (around the
top of the file where <atomic>, <chrono>, <condition_variable>, <memory>,
<mutex>, <thread> are listed) and delete the line containing `#include`
<algorithm> so only the actually used headers remain.
In `@Framework/LiveData/test/Python/scripts/adara_player_cull_events.py`:
- Around line 203-232: The code can overwrite inputs when args.output_dir is the
same directory as input files; before creating output_dir or processing files
(after resolve_input_files/classify_files but before
copy_metadata_files/downsample_event_file), add a guard that iterates over
metadata_files and event_files and checks for any file where (args.output_dir /
src.name) == src or src.parent == args.output_dir; if any match, log an error
via logger.error identifying the conflicting path and abort/return (respecting
dry_run semantics) to prevent destructive in-place overwrite; reference
resolve_input_files, classify_files, copy_metadata_files, downsample_event_file,
args.output_dir, metadata_files, and event_files when locating where to add the
check.
In `@Framework/LiveData/test/Python/scripts/adara_player_StartLiveData.py`:
- Around line 113-125: The code currently logs exceptions from captureLive() but
always proceeds to access mtd["wsOut"], which can raise KeyError and hide the
original error; modify the flow around captureLive() so that
ConfigService.setFacility(oldFacility) is executed in a finally block to
guarantee facility restoration, and only access mtd["wsOut"] (and call
wsOut.getNumberEvents()) in the try/else path after captureLive() succeeds (or
inside the try before any exception) so that wsOut is guarded; reference the
captureLive() call, mtd["wsOut"], ConfigService.setFacility(oldFacility), and
logger.error when applying this change.
In `@Framework/LiveData/test/Python/test_adara_player_server.py`:
- Around line 394-401: The test test_parseargs_returns_namespace calls
SessionServer.parse_args() which uses sys.argv by default; to avoid test-runner
flags breaking argparse, temporarily set sys.argv to a minimal list (e.g.
["prog"]) before calling SessionServer.parse_args() and restore it after; modify
the test_parseargs_returns_namespace function to save the original sys.argv,
assign a controlled list, call SessionServer.parse_args(), run the same
assertions, and finally restore the original sys.argv to ensure no side effects.
In `@Framework/LiveData/test/Python/test_adara_player.py`:
- Around line 431-433: The test uses zip([1, 2, 3], expected_names) which can
silently ignore length mismatches; update the call in the loop that invokes
Player._packet_filename to use zip(..., strict=True) so any mismatch between the
fixed index list [1, 2, 3] and expected_names will raise immediately — change
the for-loop that currently iterates over zip([1, 2, 3], expected_names) to
zip([1, 2, 3], expected_names, strict=True).
🧹 Nitpick comments (2)
Framework/LiveData/test/ISISHistoDataListenerTest.h (1)
64-94: Avoid holding the watchdog mutex while callingcancel().Line 74-77 calls
dae->cancel()while the watchdog mutex is held. Ifcancel()blocks,tearDown()will also block waiting for the mutex. Consider unlocking before the external call.♻️ Proposed change
if (!m_watchdogCv.wait_for(lock, WATCHDOG_TIMEOUT, [this] { return m_stopWatchdog; })) { // Timeout occurred auto *dae = m_daePtr.load(std::memory_order_acquire); + lock.unlock(); // avoid holding the mutex during cancel() if (dae && dae->isRunning()) { dae->cancel(); m_timedOut = true; } }Framework/LiveData/src/ADARA/utils/packet_playback/packet_player.py (1)
811-816: Preferst_mtime_nsto avoid float rounding in mtime ordering.Line 814 derives nanoseconds from
st_mtime(float). Usingst_mtime_nsavoids precision loss and makes ordering more deterministic across platforms.Suggested change
- mtime_ns = int(filePath.stat().st_mtime * 1e9) # nanoseconds since epoch + mtime_ns = filePath.stat().st_mtime_ns # nanoseconds since epoch return np.datetime64(mtime_ns, "ns")
Framework/PythonInterface/mantid/LiveData/ADARA/utils/packet_playback/packet_player.py
Show resolved
Hide resolved
Framework/LiveData/test/Python/scripts/adara_player_StartLiveData.py
Outdated
Show resolved
Hide resolved
|
Coderabbit's responses have stabilized -- they're all mostly nit-picky at this point. Please do note the |
jmborr
left a comment
jmborr
left a comment
There was a problem hiding this comment.
some comments on the documentation. I continue reading through this so expect more 😄
Framework/LiveData/src/ADARA/utils/packet_playback/adara_player_conf.yml
Outdated
Show resolved
Hide resolved
|
I want to point out, in our normal flow, the PR description will be made into the squashed commit message. Please make sure to edit the squash commit message to something shorter. In that vein, I want to make sure the helpful documentation of the PR description is saved somewhere other than this PR's description. |
ekapadi
force-pushed
the
EWM13769_live_data_playback
branch
3 times, most recently
from
6bbad0b to
8e1d116
Compare
This commit includes the following changes: * ADARA-packet replay utility implementation (see `dev-docs/source/Testing/LiveData/LiveDataPacketPlayback.rst`); * `SNSLiveEventDataListener`: allow Unix Domain Socket (UDS) addresses; * `SNSLiveEventDataListener`: duplicate `DEVICE_DESC_TYPE` packets: `warning` instead of exception. * `ISISHistoDataListenerTest`: fix test so that it can run on all platforms.
ekapadi
force-pushed
the
EWM13769_live_data_playback
branch
from
ac67d5f to
37c302b
Compare
3 participants
Description of work
This PR includes the following changes:
ADARA-packet replay utility implementation (see
dev-docs/source/Testing/LiveData/LiveDataPacketPlayback.rst);SNSLiveEventDataListener: allow Unix Domain Socket (UDS) addresses;SNSLiveEventDataListener: duplicateDEVICE_DESC_TYPEpackets:warninginstead of exception.ADARA-packet player utility
Introduction
For ORNL-facility instruments which use the ADARA system, it is possible
to record and playback an ADARA-packet stream. This ADARA-packet replay
utility was developed primarily to allow manual debugging of the
live-data listener and the live-reduction workflow. The utility should
also prove useful for debugging almost any live-data system in a
deterministic manner, even when disconnected from the instrument network
or when the beam is down.
When recording a stream, ADARA packets are saved to files. In most cases
there will be one packet per file, but this is not a requirement.
Usually it will be convenient to save the packet files for a complete
run (or part of a run) to a single directory. For debugging or
diagnostic purposes often these packet files can be obtained directly
from the DAS group (, and in this case only the utility's playback
functionality will be used). When packet-files are obtained directly,
it's important to ensure that they retain their original modification
times. Moving such files with
mvdoes this automatically,but
cp --preserve=timestampsmust be used if the files are copiedHow to use and modify
Facilities.xmlMantid reads the
Facilities.xmlfile to determine how to access eachinstrument. The instrument's network address is either in
internet-socket (INET) form, as <IPV4: port> or <IPV6: port>, or in
Unix-domain socket (UDS) form, as any accessible file-path on the
filesystem. For example, in
adara_player_conf.yml, note that thedefault socket address for the playback
serverresolves to${XDG_RUNTIME_DIR}/sock-adara_player. For testing, a loopback addresssuch as
127.0.0.1:12345may be used, but for security reasons,especially on multi-user systems, the UDS form should generally be
preferred.
When the ADARA-packet replay utility is used in
playback mode, itreplaces the real instrument. In this mode it may be necessary to
replace the instrument's address in
Facilities.xmlwith theserveraddress that will be specified either in
adara_player_conf.ymlor onthe commandline. It is also possible to simply override the
instrument's address, using the
Addressargument ofStartLiveData.Then, when the client application performs its handshaking process to
connect to the instrument and transfer data, it interacts with the
ADARA-packet replay utility instead of the real instrument.
When the ADARA-packet replay utility is used in
recordmode, the realinstrument will usually still be used, and the utility functions as a
proxy. As for
playbackmode, the instrument's address inFacilities.xmlmay be replaced with theserveraddress, but inaddition, for
recordmode it is necessary to provide the originalinstrument address as the
sourceaddress, either inadara_player_conf.ymlor on the commandline. In this situation, whenthe client application connects to the instrument, it interacts with
the ADARA-packet replay utility, which in this case connects to the real
instrument as a proxy for the client application.
ADARA-server and listener handshaking protocol
Both the
SNSLiveEventDataListener, and the ADARA-server itself,implement a handshaking protocol. In its simplest form, this protocol
involves expecting a single
CLIENT_HELLO_TYPEpacket from the clientat the start of each session. This hello-packet contains information
about the required start-time for the requested data.
Note that this ``CLIENT_HELLO_TYPE`` handshaking is the *only*
handshaking presently implemented by the ADARA-packet replay utility.
Further, the utility does not change its behavior based on the time
information present in the hello-packet, nor does it filter out
duplicate
DEVICE_DESC_TYPEpackets, if these are present. Both ofthese behaviors would be useful extensions to the utility's
implementation.
Recording a packet stream
Set up
Facilities.xml:serveraddress or note thedefault (see
adara_player_conf.yml). Next, specify a proxyinstrument in
Facilities.xml. In most cases, a useful approachwill be to comment out the existing instrument, copy its details
to an adjacent location, and then modify the address to match
the
serveraddress.Set up
adara_player_conf.yml:server.addressvalue matches that used inFacilities.xml;sourceaddress using the original instrumentaddress;
Create a base directory for the saved packet data. Note that each
time the client application connects to the proxy, a session
subdirectory will be created under this base directory.
In its own console, start the proxy server:
In another console, start your client application (e.g.
workbenchor your live-reduce Python script).When you're done recording data, type CNTL-C in the proxy server's
console.
Playing a packet-stream back
Set up
Facilities.xml:serveraddress or note thedefault (see
adara_player_conf.yml). Next, specify a proxyinstrument in
Facilities.xml. In most cases, a useful approachwill be to comment out the existing instrument, copy its details
to an adjacent location, and then modify the address to match
the
serveraddress.Set up
adara_player_conf.yml:server.addressvalue matches that used inFacilities.xml;playbackmode, thesourceaddress will not be used, soit's OK if it's still the original instrument address;
playback.packet_ordering:utility, this should be
sequence_number;yaml;playback.packet_orderingshould be treated asexperimental.
Figure out the details of the your data source directories. The
playback data can be specified by any of:
sessions.ymlfile, which includes abase_path,and then a list of
sessionsand lists of files for eachsession. This
YAMLfile would usually be played back in theorder of its sessions and file lists;
session subdirectories, as noted previously);
(note that braces may be used to form this pattern);
In its own console, start the server, in playback mode:
In another console, start your client application (e.g.
workbenchor your live-reduce Python script). In playback mode,the client will be able to successfully connect as many times as
there are session subdirectories. (For specific tests, you might
want to create additional session subdirectories with duplicate
content. In this case, it's OK just to create a link:
ln -s <existing session directory> <new session link>, for eachrequired session. For example
ln -s <base directory>/0001 <base directory>/0022, will allowsession 22 to replay what was originally recorded as session 1.)
When you're done with the playback, type CNTL-C in the playback
server's console.
Packet ordering during
playbackmodeIf the stream being played back was saved using
record, the playbackorder will be uniquely determined by the <sequence number> part of each
packet's filename. During playback, each time the client application
connects to the server, a session subdirectory will be referenced, and
the packet files in that directory will be played back in
sequence-number order. Any time it's necessary to programmatically
create a stream for deterministic playback, e.g. using a Python script
for some specific debugging scenario, this same directory and file
naming scheme should be used. Alternatively, a YAML file containing a
<base directory> and a <sessions> list of files lists can be used. In
any of these cases, it's possible to playback these session files using
another
playback.packet_orderingsetting besidessequence_numberoryaml, but that's not recommended.There are several ways to control the playback order of a packet stream.
Although an ADARA packet stream is somewhat robust to changes in the
ordering of the packets in the stream, how well this works depends on
the details of the specific packet types. (For example, changing where a
<run status> packet appears in the stream is probably a bad idea!) The
playerutility offers several distinct ways to control packet playbackorder (as specified in
adara_player_conf.ymlusing theplayback.packet_orderingkey):yaml: the order of the packet files is specified in thesessions.ymlfile; or, if nosessions.ymlfile has beenspecified, the directory order will be used;
sequence_number: the packet file's sequence number will be used(as generated by the
recordutility); this setting is recommendedwhen the packet files were generated using
recordmode, OR whencreating new packet files, or re-ordering packet files for special
tests. Using the YAML
sessions.ymlfile is usually a betterapproach when not using
recordmode.mtime: the packet file's modification time will be used;timestamp: the ADARA-packet timestamp of the packet (or of thefirst packet in a file if it contains more than one); this should be
used only in very special cases, or when there's no other
alternative. The main issue with using this approach is that often
the ADARA system starts playback by inserting configuration (or
reference) packets that may have been created months ago. In
general, the timestamps of these packets will not be updated for the
new stream. Since this type of configuration packet may be inserted
at multiple locations in the stream, the packet-timestamp based
playback order does not end up corresponding to the playback order
of the original stream. (To solve this issue: use
summarizemode,and then fill in a
sessions.ymlfile!)Using
summarizemodeGiven a set of files containing ADARA-packet data, often it can be
difficult to figure out how to set up a useful
playbacksession. Thisis especially true if the files contain multiple ADARA-packets per
file.
summarizemode was implemented in order to provide additionalinformation in this scenario. To obtain a summary of the packet data
contained in the files, the commandline syntax is:
For example, suppose we have a set of ADARA-packet files, as saved by
the ADARA instrument server, from the SNAP instrument run number 68851:
Using the names and modification times of these files, we see that the
files likely to contain event data (e.g. the larger files) were saved
with their modification times before those of the files containing the
run-status and device-descriptor data. Although the server saves the
complete data corresponding to any packet stream, we need to do some
additional work to recover what the order of that packet stream was
likely to be. In this case, the fact that the server starts a new data
file every 200 MB, and can resend the device-descriptor packets at
that file transition allows us to infer that here we probably have 3
complete 200 MB data sections, a partial ~ 100 MB section, and
possibly several other sections containing run-status and
device-descriptor information. Unfortunately, that's still not enough
information to reconstruct the stream's playback order.
Using
summarizemode, pointed at this directory, the console logoutput shows us the location of the session log, which then contains
the summary content:
The section
Suggested playback orderingat the end of the summary isan analysis of the probable file ordering based on the packet
timestamps. The difficulty for this analysis stems primarily from the
fact that several packet types, such as geometry packets, have
timestamps that correspond to the start of a proposal, which may be
months in the past. For this reason, we use a combination of the
event-timestamp ranges, followed by the control-packet timestamps (an
ad hoc hierarchy of packet types) to sort these files. Whenever
possible, files containing only metadata packets precede files
containing event packets.
The
Suggested playback orderingcan be used to create asessions.ymlfile. Each sublist under the
sessionskey corresponds to the playbackorder to use for a single session (i.e. a single client connection).
This YAML file can then be used with the
--files sessions.ymlcommandline argument of the
adara_playerapplication. WARNING: becauseof the way the ADARA-server saves its file data, often there will be
duplicate metadata sections in the group of files corresponding to a
single run. Originally the
SNSLiveEventDataListenertreated this caseas an error, but in the current version, instead this will produce a
warning in the logs.
Caveat RE ``Suggested playback ordering``:
files. Including these files in the playback generates "duplicate
device-descriptor for XX" and "XX is not a time-series property"
errors (i.e. because "XX" has not seen its device-descriptor yet);
RUN_STATUS,RUN_INFO,GEOMETRY_TYPE, andDEVICE_DESC_TYPEsections.
I'm not at all sure what is going on with this issue. Possibly the
ADARA-server is being somewhat redundant with respect to the
packet-data that it saves?
Setup for examples
In the following we assume that:
${BUILD};${SCRIPTS}:SCRIPTS=${BUILD}/../Framework/LiveData/test/Python/scripts;adara_player.py:${ADARA_PLAYER}:ADARA_PLAYER=Framework/LiveData/src/ADARA/utils/packet_playback/adara_player.py;${INPUT_DATA};${LOGS};${BUILD}/bin/launch_mantidworkbench.sh:${LOCAL_PRELOAD}${BUILD}/bin/launch_mantidworkbench.sh:${LOCAL_PYTHONPATH}Make sure that you redefine all of these for *every* console
session.
1. Build mantid, or make sure you're using a version that has an
updated SNSLiveEventDataListener. The
main issues here are:
Socket (UDS) addresses;
DEVICE_DESC_TYPEpackets now generate a warning, ratherthan raising an exception.
2. Create the test directory structure:
For use in the following, here's a separate environment variables
definition section:
3. IMPORTANT reminder: Set the
base_pathin${SCRIPTS}/adara_player_sessions.ymlto the value of${INPUT_DATA}.4. IMPORTANT reminder: Set the
logging.filenamein${ADARA_PLAYER_UTILS}/packet_playback/adara_player_conf.ymlusing the value from
${LOGS}. Note that thePIDkey will besubstituted automatically, if present in the name.
Example
playbackmode session1. In its own console: start adara_player in playback mode:
3. In another console, run the script that calls StartLiveData. Note that in this script, we
change the
Addressargument to be our defaultSERVER_ADDRESS:Example
recordmode sessionThe beam is currently down, so what can we do? It turns out quite a lot!
In this example we'll use
adara_playerinplaybackmode to mock theADARA instrument server. Then in a separate process, we'll use
adara_playerinrecordmode to record our client application'ssession. To keep things simple, we'll duplicate some of the sections
above, but with a few small changes.
1. Complete the setup as in
Setup for examples.2. a) In its own console: start
adara_playerinplaybackmode:(here we add a "-source" *suffix* to the server address):
2. b) In another console: start
adara_playerinrecordmode:(here we use the *default* server address):
3. Finally, in yet another console, run the script that calls
`StartLiveData`: (this is exactly the same as for the
``playback`` mode example).
After the client session completes, the contents of the
${OUTPUT_DATA}directory should be:
Note that in
recordmode, the default is to output all of the packetsto a single
session.adarafile, and even in the case of only oneclient connection (i.e. "session") a complete session-directory tree
will be created. The corresponding command to play this back is just
adara_player ${OUTPUT_DATA}-- only specifying the root of the tree.4. Just as a reality check, we'll verify the contents of the packet
file from the recorded session:
As can be seen, all of the packets have been combined into a single
file. Note that if it's ever required, in
adara_player_conf.yml,record.file_outputcan also be set tosingle_packetmode -- thismight be useful for special situations.
Closes EWM #11990
Reviewer
Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.
As per the review guidelines:
mantid-developersormantid-contributorsteams, add a review commentrerun cito authorize/rerun the CIGatekeeper
As per the gatekeeping guidelines: