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

No LCR when using "QD-Static, LCR, ACD&ASR control" sorting on routing plan #1166

Open
SamuelLayNZ opened this issue Aug 17, 2022 · 5 comments
Labels

Comments

@SamuelLayNZ
Copy link

When using "QD-Static, LCR, ACD&ASR control" sorting on a routing plan, prefixes listed on a routing plan static route work as intended, distributing the dialpeers by weight.

However when there is no static route created for a given prefix, it does not fall back to sorting by LCR as you would expect.
In my case it actually picks the most expensive dialpeer of the two every time, I'm not sure exactly why it picks that one first.

Here is a section of the SQL from the switch20.route function:

ORDER BY
  coalesce(v_random<=dp_force_hit_rate,false) desc,
  rpsr_priority,
  yeti_ext.rank_dns_srv(rpsr_weight) over ( partition by rpsr_priority order by rpsr_weight),
  dp_metric

I've done some testing running queries manually against the database and with the "yeti_ext.rank_dns_srv" line the expensive dialpeer is at the top of the list.
If I comment the "yeti_ext.rank_dns_srv" line out then the expensive dialpeer moves to the bottom of the list.

This makes me think that "yeti_ext.rank_dns_srv(rpsr_weight) over ( partition by rpsr_priority order by rpsr_weight)," line is returning results despite there being no static route for that prefix and preventing it from ordering by dp_metric which is based on the next rate.

@dmitry-sinina
Copy link
Contributor

could you confirm dialpeers has same priority?

@SamuelLayNZ
Copy link
Author

could you confirm dialpeers has same priority?

Yes. I've tested with same dialpeer priority and with higher/lower priority and no change.
This lines up with the SQL which doesn't take dialpeer priority into account (which it shouldn't do anyway since the routing plan static routes do this job instead)

dmitry-sinina added a commit that referenced this issue Sep 20, 2022
* src number validation
* fix priority/weight calculation for QD static sorting. refs #1166
@dmitry-sinina
Copy link
Contributor

@SamuelLayNZ could you confirm your issues fixed?

@SamuelLayNZ
Copy link
Author

@dmitry-sinina during testing in practice and with repeated routing simulations, the behaviour is changed but still not correct.

When testing calls to numbers that match the prefixes in routing plan static routes, calls are weighted as expected with 80% of calls going to Vendor 5 and 20% of calls going to Vendor 6 for example. This is good.

image

However with calls that don't match a static route by prefix, it seems to now be approximately 50/50 split when testing routing simulation repeatedly. If there were 3 Vendors it may split them evenly 33/33/33 as well, I have not tested.

See here an example of where it selects the more expensive dialpeer with Vendor 6 about 50% of the time.
image

Both dialpeers are Priority 100.
If you need anything else from me, please let me know.

@SamuelLayNZ
Copy link
Author

Looking at this again, I think I know why there is no LCR in the case where there is no routing plan static route.
COALESCE(rpsr.weight, 100) as rpsr_weight
This line is checking for weight on the routing plan static route, and if there is no RPSR it sets the weight to 100

            ORDER BY
              coalesce(v_random<=dp_force_hit_rate,false) desc,
              rpsr_priority,
              yeti_ext.rank_dns_srv(rpsr_weight) over ( partition by rpsr_priority order by rpsr_weight),
              dp_metric

Because of the yeti_ext.rank_dns_srv line, this acts as if there is a RPSR with the dialpeers all listed with weight 100 so it will pick a dialpeer 50/50 or 33.3/33.3/33.3 or 25/25/25/25 because all dialpeers have the same default weight of 100.

It never gets a chance to use the 4th ORDER BY dp_metric option to do LCR because the ordering by weight takes priority. I think we can correct this adding conditions to the ORDER BY, will see if I can come up with something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants