ECKIT-635 FAM Mock and data structures on FAM via OpenFAM lib by mcakircali · Pull Request #209 · ecmwf/eckit

mcakircali · 2025-07-02T19:01:21Z

Description

Adds a FAM-backed unordered map (plus supporting list/session/URI infrastructure) and an OpenFAM mock implementation to enable development/testing without a real OpenFAM deployment.

Introduces:

FAM data structures (FamList, FamMap, iterators, entries) and associated naming/URI/session utilities.
OpenFAM mock (shared-memory based), wired into the cmake configuration.
suite of OpenFAM/FAM unit tests
CMake options for OpenFAM/OpenFAM-mock builds

Contributor Declaration

By opening this pull request, I affirm the following:

All authors agree to the Contributor License Agreement.
The code follows the project's coding standards.
I have performed self-review and added comments where needed.
I have added or updated tests to verify that my changes are effective and functional.
I have run all existing tests and confirmed they pass.

🌦️ >> Documentation << 🌦️
https://sites.ecmwf.int/docs/dev-section/eckit/pull-requests/PR-209

new option OPENFAM (requires OpenFAM and gRPC packages) FAM source files

Copilot

Pull request overview

Copilot reviewed 57 out of 58 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/eckit/io/fam/FamMap.cc

src/eckit/io/fam/FamMapEntry.h

src/eckit/io/fam/FamMapIterator.h

src/eckit/io/fam/openfam_mock/FamMockSession.cc

src/eckit/io/fam/openfam_mock/FamMockSession.h

Copilot

Pull request overview

Copilot reviewed 57 out of 58 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/eckit/io/fam/FamMap.cc

src/eckit/io/fam/FamPath.cc

tests/io/test_fam_common.h

src/eckit/io/fam/FamSessionManager.cc

src/eckit/io/fam/FamConfig.h

src/eckit/filesystem/URI.h

src/eckit/io/fam/FamObject.cc

ECKIT-635

simondsmart · 2026-03-26T14:00:16Z

CMakeLists.txt

+
+ecbuild_add_option( FEATURE OPENFAM
+                    DEFAULT OFF
+                    REQUIRED_PACKAGES "LibUUID REQUIRED" "OpenFAM 3.2.0 REQUIRED"


You definitely do not need to repeat "REQUIRED" within the REQUIRED_PACKAGES.

Also, for something truly optional, I don't know that we want to pollute the output with error messages for optional packages.

Typically something like:

find_package(LibUUID QUIET)
find_package(OpenFAM 3.2 QUIET)

ecbuild_add_option(
FEATURE OPENFAM_MOCK
DEFAULT NOT OpenFAM_FOUND
CONDITION LibUUID_FOUND
DESCRITION "Use dummy implementation of FAM for testing"
)

ecbuild_add_option(
FEATURE OPENFAM
DEFAULT ON
CONDITION (OpenFAM_FOUND OR HAVE_OPENFAM_MOCK) AND LibUUID_FOUND)
DESCRIPTION "OPENFAM suuport"
)

(Code not tested, apologies for any typos/errors)

simondsmart · 2026-03-26T14:00:29Z

CMakeLists.txt

+                    REQUIRED_PACKAGES "LibUUID REQUIRED"
+                    DESCRIPTION "Enables OpenFAM Mock for testing and development." )
+
+if( eckit_HAVE_OPENFAM_MOCK )


The logic here is not quite right. See suggestion for previous comment.

simondsmart · 2026-03-26T14:04:37Z

src/eckit/io/fam/openfam_mock/FamMockSession.h

+namespace mock {
+
+//----------------------------------------------------------------------------------------------------------------------
+/// Capacity (@note: don't just change values)


Can you explain what these values are, where they come from, and why they should not just be changed. That is not obvious at all.

simondsmart · 2026-03-26T14:05:32Z

src/eckit/io/fam/openfam_mock/FamMockSession.h

+
+// Build time check that State fits into the shared-memory segment.
+// Leave at least 16 MiB for the data area.
+static_assert(sizeof(State) + 16 * 1024 * 1024 <= g_shm_total_size,


Why 16MiB? That is a bit of a magic number here?

simondsmart · 2026-03-26T14:09:33Z

src/eckit/io/fam/openfam_mock/FamMockSession.h

+    void reset();
+
+    /// Same as reset() but must be called while the mutex is held (e.g., during initialization).
+    void resetUnlocked();


Is this something we want to have as a public method?

simondsmart · 2026-03-26T16:18:45Z

src/eckit/io/fam/FamList.cc

+    new_object.put(length, offsetof(FamListNode, length));
+    new_object.put(data, sizeof(FamListNode), length);
+
+    // 2. Link into list: use CAS-loop to atomically update head.next


Be explicit. CAS === Compare-And-Swap.

simondsmart · 2026-03-26T16:19:32Z

src/eckit/io/fam/FamList.h

+///
+/// Supports multiple readers and writers operating concurrently without locks.
+/// Implements wait-free insertion and logical deletion, with version-based ABA detection.
+class FamList {


This class really is great.

simondsmart · 2026-03-26T16:22:10Z

src/eckit/io/fam/FamMap.cc

+
+    /// Check if key already exists
+    /// @note: This check-then-insert sequence is not atomic.
+    /// concurrent inserts of the same key may both insert resulting duplicates.


Please come and discuss. I want to understand what needs to be done to make this 'right'.

Can we write a test case that fails due to the non-atomicity.

simondsmart · 2026-03-26T16:24:22Z

src/eckit/io/fam/FamMapEntry.h

+
+    /// Encode a key-value pair into a flat buffer for storage in FamList nodes.
+    /// Layout: [key (KeySize bytes)] [value data (length bytes)]
+    static Buffer encode(const key_type& key, const void* data, size_type length) {


Do we need to construct a contiguous buffer, and write that to FAM. IS that more efficient than writing the data to FAM directly in two chunks, only doing the assembly in-situ?

simondsmart · 2026-03-26T16:25:11Z

src/eckit/io/fam/FamMapIterator.cc

+        }
+        ++bucket_;
+    }
+    // we reached the end


some of these comments are pretty uninformative.

mcakircali added 30 commits May 22, 2024 20:36

feat(Endpoint): construct from a URI

7248fbe

feat(Exception): add PermissionDenied, NotFound, and AlreadyExists

d5dd204

style(Endpoint): fix spacing

150fd3b

feat(FAM): add FamConfig

bcac18a

feat(FAM): add FamProperty

d3dd2a2

feat(FAM): add FamSessionDetail

10a3b6e

feat(FAM): add FamObjectDetail and FamRegionDetail

0db415e

feat(FAM): add FamObject

3cf06f2

feat(FAM): add FamRegion

38579c0

feat(FAM): add FamSession

8458df7

style(FAM): cleanup FamObject

7f611a9

feat(FAM): add FamName

354a5fb

feat(FAM): add FamObjectName

bf9583d

feat(FAM): add FamURIManager

073f94e

feat(FAM): add FamHandle

893543e

feat(FAM): add CMake stuff

ddb7c20

new option OPENFAM (requires OpenFAM and gRPC packages) FAM source files

feat(FAM): add basic test for region

dc7e0e3

fix(FAM): no name fam objects are valid

7548912

style(FAM): cleanup fam test

4e3b60a

feat(FAM): added FamList and FamIterator

80c3677

feat(FAM): added test for FamList

cb2b1b3

feat(Buffer): added view() method

0e7449d

chore(git): update gitignore

a0818cf

fix(FAM): minor FamList

f114c98

Merge branch 'develop' into feature/backend-openfam

a71cb1d

fix(FAM): missing add()

0a6ad77

feat(FAM): FamList size and const_iterator

8c4009e

feat(FAM): FamList test multi-thread

a0d0fa6

Merge branch 'develop' into feature/backend-openfam

8075a02

fix(FAM): logics in name and handle

a498404

Copilot started reviewing on behalf of mcakircali March 21, 2026 21:24 View session

Copilot AI reviewed Mar 21, 2026

View reviewed changes

mcakircali requested review from Copilot and simondsmart March 23, 2026 07:26

Copilot started reviewing on behalf of mcakircali March 23, 2026 07:26 View session

mcakircali changed the title ~~ECKIT-635 FAM data structures~~ ECKIT-635 FAM Mock and data structures on FAM via OpenFAM lib Mar 23, 2026

Copilot AI reviewed Mar 23, 2026

View reviewed changes

mcakircali added 13 commits March 24, 2026 09:27

fix(fam): zero fill if data is null

068a019

ECKIT-635

fix(fam): add missing include

6f82636

ECKIT-635

fix(fam): dereference const

30ae134

ECKIT-635

doc(fam): add TOCTOU comment

9ec4778

ECKIT-635

doc(fam): add map::insert duplicates

47f3cf6

ECKIT-635

fix(fam): object zero-size data

877a785

ECKIT-635

fix(fam): assert fampath parsing

6f18d27

ECKIT-635

fix(fam): fampath uuid

a098a7c

ECKIT-635

fix(fam): add missing chrono header

f862eac

ECKIT-635

test(fam): fam endpoint as env

f43017e

ECKIT-635

fix(fam): catch concurrent destroys

00aa3cd

ECKIT-635

fix(fam): cmake options

3d92483

ECKIT-635

feat(fam): add FamCommon for types

682537f

ECKIT-635

mcakircali force-pushed the feature/ECKIT-635_fam-backend-map branch from ca84fec to 682537f Compare March 24, 2026 09:33

mcakircali added 8 commits March 25, 2026 13:52

test(fam): add FamHandle

8bc1011

ECKIT-635

test(fam): add mixed

89a9d92

ECKIT-635

test(fam): split monolith fam into separate tests

64e7f9e

ECKIT-635

test(fam): move to fam directory

2bd6570

ECKIT-635

chore(fam): remove unused header

e00de34

ECKIT-635

fix(fam): reduce obj name length

008299d

ECKIT-635

feat(fam): add FamMap insertOrAssign

ba34435

ECKIT-635

fix(fam): limit mock objname size = 40

4987689

ECKIT-635

simondsmart requested changes Mar 26, 2026

View reviewed changes

Conversation

mcakircali commented Jul 2, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Declaration

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mcakircali commented Jul 2, 2025 •

edited by github-actions bot

Loading