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

enhancement(topology): Speed up vector startup with big remap transforms #20480

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Zettroke
Copy link
Contributor

@Zettroke Zettroke commented May 13, 2024

We have pretty big vrl transforms in our pipeline and we started to encounter issues with how long vector takes to startup.
Startup time was reaching of up to 10 minutes, which was troublesome.

The optimization consists of 2 thing.

  1. Cache vrl compilation results for remap, as it's outputs method is requiring to compile vrl and called many times. In the end it results in vrl compilation happening around 8 times for each remap.
  2. Parallelizing topology building.

Results

Test Baseline 1 thread 16 threads
big_conf_clean-single.yaml 11.40s 4.50s 4.47s
big_conf_clean-6.yaml 67.18s 27.24s 5.01s
big_conf_clean-chain.yaml 120.38s 31.75s 16.69s

Attaching this configs. (Yes, we really have 2kloc vrl 😂)
big_conf_clean-single.yaml.txt
big_conf_clean-6.yaml.txt
big_conf_clean-chain.yaml.txt

But there is few questions/problems I have with current implementation:

  • I'm not so keen on keeping cache in RemapConfig, as it feels kinda hacky, but I'm not sure
  • I've added rayon as a dependency, I thought about hiding it behind a feature flag, but it would be really annoying.
  • I don't like pre-cache step in compile, but I don't want to introduce par_iter into every method either.
  • Maybe there should be a cli option for limiting thread count

While startup time is not the most important metric, it's troubling when it takes minutes.
This optimization is extremely helpful for heavy use of vrl transforms and huge topologies, like my company's use case.

Hope we can resolve questions/problems and merge it.

@Zettroke Zettroke requested a review from a team as a code owner May 13, 2024 02:29
@bits-bot
Copy link

bits-bot commented May 13, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added domain: topology Anything related to Vector's topology code domain: transforms Anything related to Vector's transform components labels May 13, 2024
@Zettroke Zettroke marked this pull request as draft May 13, 2024 08:56
@Zettroke Zettroke marked this pull request as ready for review May 13, 2024 11:15
@jszwedko
Copy link
Member

Thanks for this @Zettroke ! We had the same observation that the VRL compilation was being repeated every time the outputs was calculated but hadn't been able to address it yet. We'll review this shortly.

@pront pront self-requested a review May 20, 2024 18:44
@Zettroke
Copy link
Contributor Author

There is weird test failure for topology::schema::tests::test_expanded_definition.
For some reason output doesn't get sorted. And I can't reproduce it locally
image

@bruceg
Copy link
Member

bruceg commented May 22, 2024

At a high level, this PR makes two relatively independent major changes to the remap loading code, and it would be helpful to be able to evaluate them separately. Could you untangle the compilation caching from the parallelization work? I for one would really want to address them separately.

@Zettroke
Copy link
Contributor Author

Sure thing. I'll make 2 pull requests then.

@Zettroke
Copy link
Contributor Author

Zettroke commented May 23, 2024

@bruceg Can I base parallelization branch from caching branch? Or should I make them independent? Or should I wait with it until we will finalize caching?

@bruceg
Copy link
Member

bruceg commented May 23, 2024

Make them independent if you want us to review them simultaneously, otherwise run the process with this one and the PR the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: topology Anything related to Vector's topology code domain: transforms Anything related to Vector's transform components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants