storage: add new interface to append with CT #15254

ridwanmsharif · 2024-10-31T19:17:00Z

This change does the following:

Add an interface to append a sample with CT
Update headAppender and initAppender to implement AppendWithCT
Update the RefSample to include CT
Update the scrape loop to add the CT to samples when CT is enabled

This change doesn't update the remote storage wlog watcher to make use of the new CT feild in the RefSample, but that can be done in a following PR.

We should compare using benchmarks how this compares to adding the CT to the metadata (which also goes in the WAL)

ridwanmsharif · 2024-10-31T19:41:43Z

storage/interface.go

+type AppenderWithCT interface {
+	Appender
+
+	AppendWithCT(ref SeriesRef, l labels.Labels, t int64, v float64, ct int64) (SeriesRef, error)


I added an alternative implementation that doesn't add an interface in #15255 but its uglier IMO. I had another draft that tried to use options to do this but import cycles and other issues made that approach untennable.

cc @bwplotka @ArthurSens

This change does the following: - Add an interface to append a sample with CT - Update headAppender and initAppender to implement AppendWithCT - Update the RefSample to include CT - Update the scrape loop to add the CT to samples when CT is enabled This change doesn't update the remote storage wlog watcher to make use of the new CT feild in the RefSample, but that can be done in a following PR. We should compare using benchmarks how this compares to adding the CT to the metadata (which also goes in the WAL) Signed-off-by: Ridwan Sharif <ridwanmsharif@google.com>

Signed-off-by: Ridwan Sharif <ridwanmsharif@google.com>

ridwanmsharif · 2025-01-29T23:02:55Z

/prombench main --bench.version=bench/protofirst

prombot · 2025-01-29T23:02:57Z

@ridwanmsharif is not a org member nor a collaborator and cannot execute benchmarks.

ridwanmsharif · 2025-01-29T23:04:29Z

/prombench main --bench.version=bench/protofirst

@bwplotka mind running the earlier command on the PR :)

bwplotka · 2025-01-30T07:13:47Z

This is not so simple, I got stuck with this scenario development due to metadata bug (now should be fixed, but you need to rebase), sink lack of CT checks (now fixed & released, but I need to update prombench). I need to also think how to test it e.g. we never compared metadata-wal-records on/off, so comparing one Prometheus WITHOUT metadata-wal-records feature and second WITH plus CT in sample will give us a lot of unknowns.

Will try to progress on this today.

bwplotka · 2025-01-30T10:44:03Z

/prombench main --bench.version=bench/cross-feature/ct

prombot · 2025-01-30T10:44:05Z

⏱️ Welcome to Prometheus Benchmarking Tool. ⏱️

Compared versions: PR-15254 and main

Custom benchmark version: bench/cross-feature/ct branch

After the successful deployment (check status here), the benchmarking results can be viewed at:

Available Commands:

To restart benchmark: /prombench restart main --bench.version=bench/cross-feature/ct
To stop benchmark: /prombench cancel
To print help: /prombench help

bwplotka · 2025-01-30T10:44:28Z

Quickly crafted some scenario description, likely have some bugs.. let's see.

bwplotka · 2025-01-30T10:48:36Z

/prombench restart main --bench.version=bench/cross-feature/ct

prombot · 2025-01-30T10:48:39Z

⏱️ Welcome (again) to Prometheus Benchmarking Tool. ⏱️

Compared versions: PR-15254 and main

Custom benchmark version: bench/cross-feature/ct branch

After successful deployment (check status here), the benchmarking results can be viewed at:

Available Commands:

To restart benchmark: /prombench restart main --bench.version=bench/cross-feature/ct
To stop benchmark: /prombench cancel
To print help: /prombench help

bwplotka · 2025-01-30T12:40:42Z

/prombench main --bench.version=bench/cross-feature/ct

prombot · 2025-01-30T12:40:45Z

⏱️ Welcome to Prometheus Benchmarking Tool. ⏱️

Compared versions: PR-15254 and main

Custom benchmark version: bench/cross-feature/ct branch

After the successful deployment (check status here), the benchmarking results can be viewed at:

Available Commands:

To restart benchmark: /prombench restart main --bench.version=bench/cross-feature/ct
To stop benchmark: /prombench cancel
To print help: /prombench help

bwplotka · 2025-01-30T14:07:06Z

Ok, sink does not see any CTs.


time=2025-01-30T13:59:20.854Z level=WARN msg="received remote write request with some issues" source=test-pr proto=io.prometheus.write.v2.Request series=2000 samples=2000 histograms=0 exemplars=0 details="series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"1.683411219602823\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/boom\", status=\"500\"}: no created timestamp\n,series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"+Inf\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/boom\", status=\"500\"}: no created timestamp\n,series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"0.00015000000000000001\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/foo\", status=\"200\"}: no created timestamp\n,series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"0.00022500000000000002\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/foo\", status=\"200\"}: no created timestamp\n,series codelab_api_request_duration_seconds_sum{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/foo\", status=\"200\"}: no created timestamp\n,series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"0.0001\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/foo\", status=\"200\"}: no created timestamp\n,series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"0.49878850951194753\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/boom\", status=\"500\"}: no created timestamp\n,series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"0.7481827642679213\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/boom\", status=\"500\"}: no created timestamp\n,series codelab_api_request_duration_seconds_bucket{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", le=\"1.122274146401882\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/boom\", status=\"500\"}: no created timestamp\n,series codelab_api_request_duration_seconds_count{instance=\"10.52.19.35:8081\", job=\"fake-webservers-18\", method=\"GET\", nodeName=\"gke-test-infra-nodes-15254-d01405b9-zhvd\", path=\"/api/foo\", status=\"200\"}: no created timestamp\n" series-without-unit=1980 cumulative-without-ct=1852

Looks like I forgot to enable created-timestamp-zero-ingestion feature... This really has to be another flag, quite confusing.

bwplotka · 2025-01-30T14:07:11Z

/prombench restart main --bench.version=bench/cross-feature/ct

prombot · 2025-01-30T14:07:14Z

⏱️ Welcome (again) to Prometheus Benchmarking Tool. ⏱️

Compared versions: PR-15254 and main

Custom benchmark version: bench/cross-feature/ct branch

After successful deployment (check status here), the benchmarking results can be viewed at:

Available Commands:

To restart benchmark: /prombench restart main --bench.version=bench/cross-feature/ct
To stop benchmark: /prombench cancel
To print help: /prombench help

bwplotka · 2025-01-30T14:48:20Z

bwplotka · 2025-01-30T14:55:13Z

scrape/scrape.go

+			var ct int64
+			var appWithCT storage.AppenderWithCT
+			var canEncodeCT bool
+
 			if sl.enableCTZeroIngestion {


Let's maybe create another feature flag as ageed.

bwplotka · 2025-01-31T10:41:06Z

/prombench cancel

prombot · 2025-01-31T10:41:08Z

Benchmark cancel is in progress.

Fixes #14218 and #14220 Rebased version of #15254 with improvements. This change does the following: - Change appender interface to be CT aware (optional CT) - Add created-timestamp-per-sample feature flag - Add new sample record used only if CT is appended with the sample. - Remote Write awareness of CT. Signed-off-by: Ridwan Sharif <ridwanmsharif@google.com> Signed-off-by: bwplotka <bwplotka@gmail.com>

bwplotka · 2025-02-17T13:31:06Z

Thanks! Superseded with #16046 - as discussed internally.

Fixes #14218 and #14220 Rebased version of #15254 with improvements. This change does the following: - Change appender interface to be CT aware (optional CT) - Add created-timestamp-per-sample feature flag - Add new sample record used only if CT is appended with the sample. - Remote Write awareness of CT. Signed-off-by: Ridwan Sharif <ridwanmsharif@google.com> Signed-off-by: bwplotka <bwplotka@gmail.com>

Fixes #14218 and #14220 Rebased version of #15254 with improvements. This change does the following: - Change appender interface to be CT aware (optional CT) - Add created-timestamp-per-sample feature flag - Add new sample record used only if CT is appended with the sample. - Remote Write awareness of CT. Signed-off-by: Ridwan Sharif <ridwanmsharif@google.com> Signed-off-by: bwplotka <bwplotka@gmail.com> # Conflicts: # cmd/prometheus/main.go # scrape/helpers_test.go # storage/remote/write_handler_test.go

ridwanmsharif mentioned this pull request Oct 31, 2024

append: expand the append interface to add ct to each Append call #15255

Closed

ridwanmsharif force-pushed the ridwanmsharif/ct-wal-sample-new-ct-append-interface branch from 9698da8 to 1794995 Compare October 31, 2024 19:40

ridwanmsharif commented Oct 31, 2024

View reviewed changes

github-actions bot added the stale label Dec 30, 2024

ridwanmsharif force-pushed the ridwanmsharif/ct-wal-sample-new-ct-append-interface branch from 1794995 to 10c6c0f Compare January 28, 2025 21:03

github-actions bot removed the stale label Jan 28, 2025

storage/remote: add created timestamps to the PRW2 request

154daca

Signed-off-by: Ridwan Sharif <ridwanmsharif@google.com>

ridwanmsharif force-pushed the ridwanmsharif/ct-wal-sample-new-ct-append-interface branch from 2efa134 to 154daca Compare January 29, 2025 23:02

prombot added the prombench label Jan 30, 2025

bwplotka reviewed Jan 30, 2025

View reviewed changes

bwplotka mentioned this pull request Feb 17, 2025

ct: Support CTs in WAL; change sample record; use in PRW 2.0 #16046

Draft

bwplotka closed this Feb 17, 2025

storage: add new interface to append with CT #15254

storage: add new interface to append with CT #15254

Uh oh!

Conversation

ridwanmsharif commented Oct 31, 2024

Uh oh!

ridwanmsharif Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

ridwanmsharif commented Jan 29, 2025

Uh oh!

prombot commented Jan 29, 2025

Uh oh!

ridwanmsharif commented Jan 29, 2025

Uh oh!

bwplotka commented Jan 30, 2025

Uh oh!

bwplotka commented Jan 30, 2025

Uh oh!

prombot commented Jan 30, 2025

Uh oh!

bwplotka commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bwplotka commented Jan 30, 2025

Uh oh!

prombot commented Jan 30, 2025

Uh oh!

bwplotka commented Jan 30, 2025

Uh oh!

prombot commented Jan 30, 2025

Uh oh!

bwplotka commented Jan 30, 2025

Uh oh!

bwplotka commented Jan 30, 2025

Uh oh!

prombot commented Jan 30, 2025

Uh oh!

bwplotka commented Jan 30, 2025

Uh oh!

bwplotka Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

bwplotka commented Jan 31, 2025

Uh oh!

prombot commented Jan 31, 2025

Uh oh!

bwplotka commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bwplotka commented Jan 30, 2025 •

edited

Loading

bwplotka commented Feb 17, 2025 •

edited

Loading