Skip to content

Publish on-demand log JSON atomically - #13

Draft
okt-limonikas wants to merge 2 commits into
ts-factory:mainfrom
okt-limonikas:fix/atomic-log-json-generation
Draft

Publish on-demand log JSON atomically#13
okt-limonikas wants to merge 2 commits into
ts-factory:mainfrom
okt-limonikas:fix/atomic-log-json-generation

Conversation

@okt-limonikas

@okt-limonikas okt-limonikas commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix intermittent empty, truncated, or invalid JSON responses when multiple
clients request an uncached log node at the same time.

The converter now publishes complete JSON files atomically, while the CGI
handler ensures that only one request generates a given target.

Related to:

  1. [Bug]: Logs not loading from first time bublik-ui#573
  2. fix(log): retry transient JSON generation failures bublik-ui#621

Problem

rgt-xml2json previously wrote directly to the public JSON path. Apache could
serve that file before serialization finished. Concurrent requests could also
start multiple converters that truncated and wrote the same file.

This appeared in the UI as an empty or unavailable selected test. Reloading or
navigating away and back worked after generation had completed.

You can test via this small bash script by pointing at some URL from apache log server

#!/usr/bin/env bash
set -euo pipefail

if [[ $# -lt 1 ]]; then
  echo "Usage: $0 <base_url> [start_id] [end_id] [concurrency]"
  echo
  echo "Example:"
  echo "  $0 http://127.0.0.1:42000/logs/example/json 1 100 20"
  exit 1
fi

base_url="${1%/}"
start_id="${2:-1}"
end_id="${3:-100}"
concurrency="${4:-20}"

output_dir="$(mktemp -d)"
trap 'rm -rf "$output_dir"' EXIT

total_valid=0
total_invalid=0

for id in $(seq "$start_id" "$end_id"); do
  echo "Testing node_id$id..."

  node_dir="$output_dir/node_id$id"
  mkdir -p "$node_dir"

  url="${base_url}/node_id${id}.json"

  # Fire concurrent requests
  for ((i = 0; i < concurrency; i++)); do
    (
      curl -sSL "$url" -o "$node_dir/response-$i.json" ||
        rm -f "$node_dir/response-$i.json"
    ) &
  done
  wait

  valid=0
  invalid=0

  for file in "$node_dir"/response-*.json; do
    if [[ ! -f "$file" ]]; then
      ((++invalid))
      ((++total_invalid))
      continue
    fi

    if jq -e 'type == "object" and has("root")' "$file" >/dev/null 2>&1; then
      ((++valid))
      ((++total_valid))
    else
      size=$(wc -c <"$file")
      echo "  invalid: $(basename "$file") (${size} bytes)"
      ((++invalid))
      ((++total_invalid))
    fi
  done

  echo "  node_id$id: $valid valid, $invalid invalid"
done

echo
echo "Overall Summary"
echo "---------------"
echo "Valid:   $total_valid"
echo "Invalid: $total_invalid"
echo "Total:   $((total_valid + total_invalid))"

Example output

Be aware that this is flaky and some behave correctly and some fail so test with multiple nodes (large, small, etc...)

Like for this url https://ts-factory.io/logs/dpdk-ethdev-ts/2024/03/24/virtio_virtio:tuor-cbs-speed-stack-00:14:19/json/

./check_nodes.sh https://ts-factory.io/logs/dpdk-ethdev-ts/2024/03/24/virtio_virtio:tuor-cbs-speed-stack-00:14:19/json/ 1 100 20

Testing node_id1...
  node_id1: 20 valid, 0 invalid
Testing node_id2...
  node_id2: 20 valid, 0 invalid
Testing node_id3...
  node_id3: 20 valid, 0 invalid
Testing node_id4...
  node_id4: 20 valid, 0 invalid
Testing node_id5...
  node_id5: 20 valid, 0 invalid
Testing node_id6...
  node_id6: 20 valid, 0 invalid
Testing node_id7...
  node_id7: 20 valid, 0 invalid
Testing node_id8...
  node_id8: 20 valid, 0 invalid
Testing node_id9...
  node_id9: 20 valid, 0 invalid
Testing node_id10...
  node_id10: 20 valid, 0 invalid
Testing node_id11...
  node_id11: 20 valid, 0 invalid
Testing node_id12...
  node_id12: 20 valid, 0 invalid
Testing node_id13...
  node_id13: 20 valid, 0 invalid
Testing node_id14...
  node_id14: 20 valid, 0 invalid
Testing node_id15...
  invalid: response-0.json (   20480 bytes)
  invalid: response-10.json (  102400 bytes)
  invalid: response-5.json (   77824 bytes)
  invalid: response-7.json (  122501 bytes)
  node_id15: 16 valid, 4 invalid
Testing node_id16...
  invalid: response-12.json (   12288 bytes)
  invalid: response-15.json (   12288 bytes)
  invalid: response-17.json (    8192 bytes)
  invalid: response-8.json (       0 bytes)
  node_id16: 16 valid, 4 invalid
Testing node_id17...
  node_id17: 20 valid, 0 invalid
Testing node_id18...
  node_id18: 20 valid, 0 invalid
Testing node_id19...
  node_id19: 20 valid, 0 invalid

Changes

tools/rgt: publish generated JSON atomically

  • Write node JSON and tree.json to hidden files in the destination directory.
  • Close each file and publish it with an atomic same-directory rename().
  • Preserve an existing output mode, or use 0666 & ~umask for a new file.
  • Lock active temporary files and remove abandoned files on a later conversion.

Apache now sees either no public file or one complete JSON document. Cleanup
does not remove a temporary file that is still being written.

tools/log_server: serialize on-demand generation

  • Canonicalize the requested path and reject paths outside the logs root.
  • Serialize generation through a bounded pool of 256 flock files.
  • Recheck the target after acquiring the lock so waiting requests reuse it.
  • Return explicit HTTP 302, 404, and 500 responses without exposing converter
    diagnostics to clients.

The bounded pool prevents attacker-controlled request paths from creating an
unlimited number of persistent lock files. Hash collisions only serialize
unrelated generation requests.

On-demand JSON files were written directly to their public paths.
Apache could serve a file as soon as it was opened, so concurrent
clients sometimes received empty or truncated JSON.

Write node files and tree.json to same-directory temporary files and
rename them only after a successful close. Preserve output permissions
and reap temporary files abandoned by interrupted conversions without
disturbing active writers.

Link: ts-factory/bublik-ui#573
Link: ts-factory/bublik-ui#621
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
@okt-limonikas
okt-limonikas force-pushed the fix/atomic-log-json-generation branch from 834bec5 to af190e1 Compare August 4, 2026 15:45
@okt-limonikas
okt-limonikas marked this pull request as ready for review August 4, 2026 15:47
@okt-limonikas
okt-limonikas marked this pull request as draft August 4, 2026 16:14
Concurrent requests for a missing JSON file could start multiple
converters for the same destination. This wasted resources and made the
partial-file race more likely.

Canonicalize requested paths and serialize generation through a bounded
256-slot lock pool. Recheck the target under the lock so waiters reuse
the completed file, return explicit HTTP statuses, and keep generator
diagnostics out of client responses.

Link: ts-factory/bublik-ui#573
Link: ts-factory/bublik-ui#621
Signed-off-by: Danil Kostromin <danil.kostromin@icloud.com>
@okt-limonikas
okt-limonikas force-pushed the fix/atomic-log-json-generation branch from af190e1 to 569be08 Compare August 4, 2026 16:20
# Outputs:
# HTTP response
#############################
function internal_error_exit() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dislike that really different changes are mixed in one changeset. It makes the review harder.
Please, factor out introduction of these functions into separate patch.

request_file=$(realpath -m -- "${request_file}") || internal_error_exit
case "${request_file}" in
"${canonical_root}"/* ) ;;
* ) not_found_exit ;;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logically it is a separate enhancement not directly related to locking and serialization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants