Skip to content

fix(dnstap source): increase request limit for TCP dnstap source#23448

Merged
pront merged 4 commits intovectordotdev:masterfrom
esensar:fix/tcp-dnstap-throughput-reduction
Aug 6, 2025
Merged

fix(dnstap source): increase request limit for TCP dnstap source#23448
pront merged 4 commits intovectordotdev:masterfrom
esensar:fix/tcp-dnstap-throughput-reduction

Conversation

@esensar
Copy link
Contributor

@esensar esensar commented Jul 28, 2025

Summary

Uses max_frame_handling_tasks setting to increase the default request limit. By default, max_frame_handling_tasks is 1000 and since the code for this component only handles 1 event for each permit, this translates well into this usage. This greatly reduces time lost by re-acquiring permits and waiting for connections that produce no data to drop the permits.

Vector configuration

api:
  enabled: true
  address: "0.0.0.0:8686"

sources:
  dnstap_tcp:
    type: "dnstap"
    mode: "tcp"
    address: "0.0.0.0:59001"

sinks:
  console:
    inputs: ["dnstap_tcp"]
    type: "blackhole"

How did you test this PR?

Started Vector with the above configuration, connected (THREAD COUNT - 1) dnstap clients that send no data and then connected 64 more. Observing via vector top, before #23123, this managed 200-300k events/sec, but it often produced errors and crashed connections. After #23123, this scenario dropped to <10k events/sec. After this fix, it is close to the original (still a bit less, but still in 200-300k events/sec range) and no errors appear. This can further be controlled with max_frame_handling_tasks, which is already used in this component - default is 1000, but performance can further be improved, although it doesn't make much difference if it is already higher than connection count.

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the no-changelog label to this PR.

References

Notes

  • Please read our Vector contributor resources.
  • Do not hesitate to use @vectordotdev/vector to reach out to us regarding this PR.
  • Some CI checks run only after we manually approve them.
    • We recommend adding a pre-push hook, please see this template.
    • Alternatively, we recommend running the following locally before pushing to the remote branch:
      • cargo fmt --all
      • cargo clippy --workspace --all-targets -- -D warnings
      • cargo nextest run --workspace (alternatively, you can run cargo test --all)
  • After a review is requested, please avoid force pushes to help us review incrementally.
    • Feel free to push as many commits as you want. They will be squashed into one before merging.
    • For example, you can run git merge origin master and git push.
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run cargo vdev build licenses to regenerate the license inventory and commit the changes (if any). More details here.

@github-actions github-actions bot added the domain: sources Anything related to the Vector's sources label Jul 28, 2025
@esensar
Copy link
Contributor Author

esensar commented Aug 6, 2025

@jszwedko Could this get in before the release? It is a small change, but it prevents the issue that forces some users to use 0.47.0

@pront
Copy link
Member

pront commented Aug 6, 2025

@jszwedko Could this get in before the release? It is a small change, but it prevents the issue that forces some users to use 0.47.0

Hi @esensar, is it ready for review? The release is scheduled for next week but we can probably include this PR.

@esensar esensar marked this pull request as ready for review August 6, 2025 13:26
@esensar esensar requested a review from a team as a code owner August 6, 2025 13:26
@esensar
Copy link
Contributor Author

esensar commented Aug 6, 2025

Sorry, I must have left it a draft by accident. It is ready, I will add a changelog.

@pront pront enabled auto-merge August 6, 2025 13:51
@pront pront added this pull request to the merge queue Aug 6, 2025
Merged via the queue into vectordotdev:master with commit e7a3126 Aug 6, 2025
96 checks passed
esensar added a commit to esensar/vector that referenced this pull request Aug 6, 2025
@esensar esensar deleted the fix/tcp-dnstap-throughput-reduction branch August 6, 2025 15:21
github-merge-queue bot pushed a commit that referenced this pull request Aug 6, 2025
…size` (#23537)

* chore(dnstap source): change type of `max_frame_handling_tasks` to `usize`

Related: #23448 (comment)

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

Labels

domain: sources Anything related to the Vector's sources

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing or throttled events with dnstap TCP source on v0.48.0

2 participants