ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.31k stars 1.63k forks source link

#1526 Benchmark for startup when merging multiple config files #1856

Closed ks1990cn closed 6 months ago

ks1990cn commented 8 months ago

Fixes #1526

Adding benchmark for startup. We can replicate issue or verify with this PR

Unable to replicate application down with 600 dummy routes.

Proposed Changes

raman-m commented 8 months ago

Wow! Tanmay! I'm impressed!

But why do you create new PR(s) having ignored the work on other ones? Seems you need to work on #1842 ... right?

I would say the team can review this PR after work completion of the previous PR #1842 . Why not to focus on the one PR?

@ggnaegi FYI

ks1990cn commented 8 months ago

@raman-m , I was waiting for @ggnaegi reply on PR. Meanwhile I started trying with different issues 😄

raman-m commented 6 months ago

Proposed Changes

  • Current benchmarking is done for 600+ dummy routes across 2 files, more file and more dummy routes can be extended easily.

Tanmay, seems you didn't understand clearly the user scenario:

Previously I include seven different configurations files with the routes of different projects API, The problem occurred when a new file is added with more than 80 routes in it.

First, we don't know how many routes were defined in these old 7 files... maybe 100, but maybe 500, mauve only 7 😄 I asked the author to attach original config files, but she didn't care....

My understanding of #1526 is the following:

Your benchmark is great! And seems a bit more development should be provided.

@ggnaegi Could you review this PR please, and advise on benchmark quality? I believe it is useful, but it needs some refactoring.

raman-m commented 6 months ago

⚠️ I've rebased the branch onto develop!

ks1990cn commented 6 months ago

Sorry for inconvenience, I'll be raising PR again. Something went wrong with my develop branch and was getting unable to find latest commits on it.

raman-m commented 6 months ago

With develop or feature branch?

Please keep your develop identical to head repo develop! Use the button Sync fork!

Regarding feature branch... As I said I've rebased it onto develop! What can be wrong here? Your old local commits are not compatible to new commits on origin. To fix that you have to re-check out the branch:

git switch develop
git branch -D feature
git fetch --all
git checkout feature

Clear?

ks1990cn commented 6 months ago

I use same, sync branch sir. But somehow I was getting unnnecessary commits in that and I was ahead of 4-5 commits on develop. On merging that to my this branch may have caused issue.

So I decided to refork!

ks1990cn commented 6 months ago

I am sorry sir @raman-m .