Conversation
983bda9 to
2c4b79d
Compare
srebhan
left a comment
There was a problem hiding this comment.
Can we please keep the schema of renaming the existing file? If you detect a collision )(e.g. myname-2026-09-07-1788773604.parquet), use filepath.Glob to find all renaming target files (e.g. myname-2026-09-07-1788773604_*.parquet), then detect the last entry (e.g. myname-2026-09-07-1788773604_004.parquet), parse the number (e.g. 004) and increase it. This is your new target. It's still racy though as another process could create that file between you creating the list and actually renaming the file but I guess that's only avoidable if you use a uuid or something as suffix...
|
well I was hoping we wouldn't need to add all that logic if we could just order the schema columns in a deterministic order #19556 |
|
Sorry, I do not understand what this has to do with column ordering. From what I understand Telegraf currently creates a new file if the file already exists and moves away the old one. You brought up a valid corner case that the renaming can fail as the logic to determine the target might lead to the same filename as the current (or another) file. I thought this PR is fixing the name-collision issue, isn't it? |
instead of checking the schema of the existing file, the schema will match if we order the columns deterministically and if the schema columns haven't changed. which is why I said #19556 may fix the issue
so that's not what I saw during testing. this is running on rootless podman. $ docker pull telegraf:1.39.3-alpine
[...]
$ cat telegraf.conf
[agent]
interval = "100ms"
flush_interval = "100ms"
flush_jitter = "0s"
collection_jitter = "0s"
round_interval = false
omit_hostname = true
[[inputs.mock]]
metric_name = "cpu"
[[inputs.mock.step]]
name = "value"
start = 0.0
step = 1.0
[[outputs.parquet]]
directory = "/out"run one container$ rm -rf ./out/*
$ podman run --rm --entrypoint telegraf \
-v ./telegraf.conf:/etc/telegraf/telegraf.conf:ro \
-v ./out:/out \
docker.io/library/telegraf:1.39.3-alpine \
--config /etc/telegraf/telegraf.conf --once
[...]
$ ls out/
cpu-2026-09-08-1788883487.parquet10 runs$ rm -rf ./out/*
$ for i in $(seq 1 10); do
podman run --rm --entrypoint telegraf \
-v ./telegraf.conf:/etc/telegraf/telegraf.conf:ro \
-v ./out:/out \
docker.io/library/telegraf:1.39.3-alpine \
--config /etc/telegraf/telegraf.conf --once >/dev/null 2>&1
done
[...]
$ ls out/
cpu-2026-09-08-1788883594.parquet cpu-2026-09-08-1788883595.parquet10 runs this branch$ rm -rf ./out/*
$ make build
CGO_ENABLED=0 go build -tags "" -ldflags " -X github.com/influxdata/telegraf/internal.Commit=1b9fbaee -X github.com/influxdata/telegraf/internal.Branch=parquet-07-no-overwrite -X github.com/influxdata/telegraf/internal.Version=1.41.0-1b9fbaee" ./cmd/telegraf
$ ./telegraf --version
Telegraf 1.41.0-1b9fbaee (git: parquet-07-no-overwrite@1b9fbaee)
$ cat telegraf.conf
[agent]
interval = "100ms"
flush_interval = "100ms"
flush_jitter = "0s"
collection_jitter = "0s"
round_interval = false
omit_hostname = true
[[inputs.mock]]
metric_name = "cpu"
[[inputs.mock.step]]
name = "value"
start = 0.0
step = 1.0
[[outputs.parquet]]
directory = "./out"
$ for i in $(seq 1 10); do
./telegraf --config telegraf.conf --once >/dev/null 2>&1
done
$ ls ./out
cpu-2026-09-08-1788886996-1.parquet cpu-2026-09-08-1788886996-4.parquet cpu-2026-09-08-1788886996-7.parquet cpu-2026-09-08-1788886996.parquet
cpu-2026-09-08-1788886996-2.parquet cpu-2026-09-08-1788886996-5.parquet cpu-2026-09-08-1788886996-8.parquet
cpu-2026-09-08-1788886996-3.parquet cpu-2026-09-08-1788886996-6.parquet cpu-2026-09-08-1788886996-9.parquet |
2c4b79d to
1b9fbae
Compare
|
@81reap I now see what you are talking about. What you want to do is to write to the same directory with multiple Telegraf agents. Is this correct? |
|
yep! that's why #19563 I try to have multiple writes writing to the same file and schema in the same timestamp. it would be like multiple machines (CPU being collected on each machine but replicated to one machine) or agents writing to the same thing (multiple NVIDA gpus on one machine). that's why i've been trying to fix all these things in the stack where data isn't lost when it doesn't match the schema or isn't lost when multiple things are writing to it. |
|
@81reap I changed the PR title and description to reflect what you are trying to do. I suggest to change the logic for the filename to create files using a For creating the UUID you can use e.g. this function and get the |
1b9fbae to
ca153a6
Compare
|
I didn't go with UUID as that's what the current README calls out as the expected behaviour. I suppose it does match Hive partitioning standards so end users may be more familiar with it. I've update the PR, although this is now a breaking change for end users depending on how they consume the output parquets. But just to clarify multiple agents is only one way to trigger it and currently hasn't been supported by TeleGraf and is a The bigger issue is what the original title called out :: when TeleGraf rotates files, it looses data. I suppose using a for loop made that more confusing than clear. This time I use Hopefully this does a better job of showing how we're breaking this gurantee. telegraf/plugins/outputs/parquet/README.md Lines 87 to 94 in 7b4316a one container$ cat telegraf.conf
[agent]
interval = "100ms"
flush_interval = "100ms"
flush_jitter = "0s"
collection_jitter = "0s"
round_interval = false
omit_hostname = true
[[inputs.mock]]
metric_name = "cpu"
[[inputs.mock.step]]
name = "value"
start = 0.0
step = 1.0
[[outputs.parquet]]
directory = "/out"
$ rm -rf ./out/*
$ timeout -s INT 10 podman run --rm --entrypoint telegraf \
-v ./telegraf.conf:/etc/telegraf/telegraf.conf:ro \
-v ./out:/out \
docker.io/library/telegraf:1.39.3-alpine \
--config /etc/telegraf/telegraf.conf
[...]
$ ls out/
cpu-2026-09-17-1789651240.parquet
$ duckdb -c "select count(*) as rows, min(value) as first, max(value) as last from read_parquet('out/*.parquet')"
┌───────┬────────┬────────┐
│ rows │ first │ last │
│ int64 │ double │ double │
├───────┼────────┼────────┤
│ 99 │ 0.0 │ 98.0 │
└───────┴────────┴────────┘99 rows, last value 98. nothing lost. one container, rotation_interval = "100ms"$ tail -3 telegraf.conf
[[outputs.parquet]]
directory = "/out"
rotation_interval = "100ms" # this is the new line
$ rm -rf ./out/*
$ timeout -s INT 10 podman run --rm --entrypoint telegraf \
-v ./telegraf.conf:/etc/telegraf/telegraf.conf:ro \
-v ./out:/out \
docker.io/library/telegraf:1.39.3-alpine \
--config /etc/telegraf/telegraf.conf
[...]
$ ls out/
cpu-2026-09-17-1789651255.parquet cpu-2026-09-17-1789651260.parquet
cpu-2026-09-17-1789651256.parquet cpu-2026-09-17-1789651261.parquet
cpu-2026-09-17-1789651257.parquet cpu-2026-09-17-1789651262.parquet
cpu-2026-09-17-1789651258.parquet cpu-2026-09-17-1789651263.parquet
cpu-2026-09-17-1789651259.parquet cpu-2026-09-17-1789651264.parquet
$ duckdb -c "select count(*) as rows, min(value) as first, max(value) as last from read_parquet('out/*.parquet')"
┌───────┬────────┬────────┐
│ rows │ first │ last │
│ int64 │ double │ double │
├───────┼────────┼────────┤
│ 10 │ 19.0 │ 98.0 │
└───────┴────────┴────────┘same 10 seconds, same one process. 10 rows out of 99. |
79c3820 to
8ccd151
Compare
|
added another also aligned on the new PR title, I just wanted to make sure that I wasn't only communicating multiple writers but the a bigger dataloss issue also since this uses |
Telegraf currently tries to move aside an existing file of the same name by adding the current time. This causes a conflict when both the moved file and the new file are named into the same second causing a no-op and `os.Create` would overwrite and lose data. Bad inputs that cause a restart loop would trigger this issue per cycle. Files are now opened with `O_EXCL` and a numeric suffix is added until an unused name is found.
8ccd151 to
4070681
Compare
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
|
ci failure is unrelated to my changes |
The current code creates files with a
<measurement>-<YYYY-MM-DD>-<unix-seconds>.parquetpattern using the timestamp whenWritewas called. If the file already exists the existing file is renamed using a new, current timestamp and the same pattern.At this, the code assumes only one writer per directory which should minimize the filename collisions to restarts of Telegraf. However, as #19563 describes, having multiple writers, i.e. multiple plugin instances and/or multiple telegraf instances, writing to the same directory will increase the risk of filename collisions and creates the risk of multiple renames target the same filename because they might happen at the same time. This leads to data loss as one writer will override the data of other writers on collision.
This PR changes the logic when creating a new writer to create files of the pattern
<measurement>-<YYYYMMDDhhmmss>-<uuid v6>.parquetwhere an additional (time-based) UUID is added to the filename to ensure unique filenames. This also prevents the risk of filename collisions for multiple, independent writers.Checklist
Related issues
resolves #19563