Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOCS-7980] Remove buffer in OP docs #23270

Merged
merged 6 commits into from
Jun 3, 2024
Merged

[DOCS-7980] Remove buffer in OP docs #23270

merged 6 commits into from
Jun 3, 2024

Conversation

maycmlee
Copy link
Contributor

@maycmlee maycmlee commented May 20, 2024

What does this PR do? What is the motivation?

There isn't a buffer option for OP 2.0 so removing it from the docs.

DOCS-7980

Merge instructions

  • Please merge after reviewing

Additional notes

@maycmlee maycmlee added the WORK IN PROGRESS No review needed, it's a wip ;) label May 20, 2024
@maycmlee maycmlee requested a review from a team as a code owner May 20, 2024 16:46
@@ -51,7 +51,7 @@ Observability Pipelines Worker runs on modern CPU architectures. X86_64 architec

### Memory sizing

Due to Observability Pipelines Worker's affine type system, memory is rarely constrained for Observability Pipelines Worker workloads. Therefore, Datadog recommends ≥2 GiB of memory per vCPU minimum. Memory usage increases with the number of sinks due to the in-memory buffering and batching. If you have a lot of sinks, consider increasing the memory or switching to disk buffers.
Due to Observability Pipelines Worker's affine type system, memory is rarely constrained for Observability Pipelines Worker workloads. Therefore, Datadog recommends ≥2 GiB of memory per vCPU minimum. Memory usage increases with the number of sinks due to the in-memory buffering and batching. If you have a lot of sinks, consider increasing the memory.

### Disk sizing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ckelner do we want to remove this "Disk Sizing" section as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Per https://vector.dev/docs/setup/going-to-prod/sizing/#disks it states:

Sizing disks is only relevant if you’re using Vector’s disk buffers.

But I feel like we should have some kind of guidance here, even w/o disk buffers.
Let me ask the OP backroom, I have a thread on this topic.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maycmlee I think for "Disk Sizing" we just say "30GB minimum" and leave it at that.

Copy link
Contributor Author

@maycmlee maycmlee May 21, 2024

Choose a reason for hiding this comment

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

@ckelner could you let me know if the update below is what you are suggesting? (the commit) I'm assuming the example caclulation is still correct?

For example, an 8 vCPU machine would require 288 GiB of disk space (10 MiB * 60 seconds * 60 minutes * 8 vCPUs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chatted on zoom. Remove the entire section and just add the minimum required sentence.

Copy link
Contributor

github-actions bot commented May 20, 2024

Preview links (active after the build_preview check completes)

Modified Files

Copy link
Contributor

@ckelner ckelner left a comment

Choose a reason for hiding this comment

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

Just commenting for now, lets wait on OP Eng to comment on this: https://github.com/DataDog/documentation/pull/23270/files#r1607013668

@@ -89,29 +88,6 @@ Data processing is shifted off your nodes and onto remote aggregator nodes. Remo

See [Aggregator Architecture][5] for more details.

## Buffering data
Copy link
Contributor

Choose a reason for hiding this comment

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

I think removing this for now is good, but we should write up something about the "built in default" buffering (and remove language about being able to configure it).

We can probably pull something together from here: https://vector.dev/docs/about/under-the-hood/architecture/buffering-model/ then likely have eng review it.

I can draft something in a google doc if you like

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, if you can draft something that'd be great. Thanks!

Copy link
Contributor

@ckelner ckelner May 21, 2024

Choose a reason for hiding this comment

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

EDIT: Disregard - old docs

Check it out here May: https://docs.google.com/document/d/1UKspJYeXbvY5tFdlJUJgjvJ2ngLfg36MAkKVz6YCQvo/edit

Want to post in the #obs-pipelines-backroom channel for a look?~

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh jeez! These are the "legacy" 1.0 docs. We can disregard. Leave this as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry that was my fault, I thought I had stuck to the current docs. I reverted the change.

Copy link
Contributor

@ckelner ckelner left a comment

Choose a reason for hiding this comment

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

Let's update disk buffering, and let's revert the delete for the legacy docs.

@@ -89,29 +88,6 @@ Data processing is shifted off your nodes and onto remote aggregator nodes. Remo

See [Aggregator Architecture][5] for more details.

## Buffering data
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh jeez! These are the "legacy" 1.0 docs. We can disregard. Leave this as-is.

@@ -51,7 +51,7 @@ Observability Pipelines Worker runs on modern CPU architectures. X86_64 architec

### Memory sizing

Due to Observability Pipelines Worker's affine type system, memory is rarely constrained for Observability Pipelines Worker workloads. Therefore, Datadog recommends ≥2 GiB of memory per vCPU minimum. Memory usage increases with the number of sinks due to the in-memory buffering and batching. If you have a lot of sinks, consider increasing the memory or switching to disk buffers.
Due to Observability Pipelines Worker's affine type system, memory is rarely constrained for Observability Pipelines Worker workloads. Therefore, Datadog recommends ≥2 GiB of memory per vCPU minimum. Memory usage increases with the number of sinks due to the in-memory buffering and batching. If you have a lot of sinks, consider increasing the memory.

### Disk sizing
Copy link
Contributor

Choose a reason for hiding this comment

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

@maycmlee I think for "Disk Sizing" we just say "30GB minimum" and leave it at that.

Copy link
Contributor

@ckelner ckelner left a comment

Choose a reason for hiding this comment

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

just thought of something, we don't call them sinks anymore right, should we change to destinations?

@maycmlee maycmlee requested a review from ckelner May 23, 2024 13:27
@maycmlee maycmlee removed the WORK IN PROGRESS No review needed, it's a wip ;) label Jun 3, 2024
@maycmlee maycmlee merged commit 2a0f492 into master Jun 3, 2024
18 checks passed
@maycmlee maycmlee deleted the may/op-remove-buffer branch June 3, 2024 16:14
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.

None yet

3 participants