[arkjedrz]

• Set permissions of stored files to 640.
• Make store operation atomic per file.
  • Temp file + fsync + rename.

Resolves #412

[github-actions]

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Starting local Bazel server (8.7.0) and connecting to it...
INFO: Invocation ID: 64c63869-8754-4e13-aaee-67c2215cba92
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 2 packages loaded
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

Analyzing: target //:license-check (88 packages loaded, 14 targets configured)

Analyzing: target //:license-check (146 packages loaded, 3240 targets configured)

Analyzing: target //:license-check (156 packages loaded, 8141 targets configured)

Analyzing: target //:license-check (162 packages loaded, 9666 targets configured)

Analyzing: target //:license-check (163 packages loaded, 9786 targets configured)

Analyzing: target //:license-check (166 packages loaded, 11376 targets configured)

INFO: Analyzed target //:license-check (168 packages loaded, 15897 targets configured).
[11 / 16] [Prepa] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 50 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 24.267s, Critical Path: 2.41s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
[main] INFO Querying Eclipse Foundation for license data for 42 items.
[main] INFO Found 32 items.
[main] INFO Querying ClearlyDefined for license data for 10 items.
[main] INFO Found 10 items.
[main] INFO License information could not be automatically verified for the following content:
[main] INFO 
[main] INFO crate/cratesio/-/lazy_static/1.5.1
[main] INFO 
[main] INFO This content is either not correctly mapped by the system, or requires review.

[github-actions]

Documentation preview for this pull request is available at:
pr-503: https://eclipse-score.github.io/persistency/pr-503/

[Ahmed-Elsaka-JC]

I still see race condition here

We need to check if baselibs providing open,fopen APIs to avoid this race condition

Expectation to have something like following :

#include
#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>

auto file_deleter = [](std::FILE* f) {
if (f != nullptr) {
(void)std::fclose(f);
}
};

using FilePtr = std::unique_ptr<std::FILE, decltype(file_deleter)>;

const int fd = ::open(
path.Native().c_str(),
O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC,
S_IRUSR | S_IWUSR | S_IRGRP); // 0640

if (fd == -1) {
logger_->LogError() << "Failed to open file " << path;
return score::MakeUnexpected(ErrorCode::PhysicalStorageFailure);
}

std::FILE* raw_file = ::fdopen(fd, "wb");

if (raw_file == nullptr) {
(void)::close(fd);
logger_->LogError() << "Failed to create file stream " << path;
return score::MakeUnexpected(ErrorCode::PhysicalStorageFailure);
}

FilePtr file{raw_file, file_deleter};

[arkjedrz]

I shifted the focus from permissions to atomic operation.

• AtomicUpdate exists in baselibs, but cannot be used due to permissions not being set.
• Using two files will never be fully atomic. Not sure how this should be handled - a single file with a data checksum at the top, then data?

[Ahmed-Elsaka-JC]

I shifted the focus from permissions to atomic operation.

• AtomicUpdate exists in baselibs, but cannot be used due to permissions not being set.
• Using two files will never be fully atomic. Not sure how this should be handled - a single file with a data checksum at the top, then data?

i would say having single file contains hash and kvs data is a good idea , let us try to discuss this topic coming meeting.

[PandaeDo]

Fine for me from perspective of safety&quality

[karolnowak]

Looks good to me.

[Ahmed-Elsaka-JC]

from CPP prespective LGFM

[vinodreddy-g]

LGTM