Re: [PR] docs: fix priority values and permalink in FAQ doc [apisix-ingress-controller]
kayx23 commented on code in PR #2712: URL: https://github.com/apache/apisix-ingress-controller/pull/2712#discussion_r3239398369 ## docs/en/latest/FAQ.md: ## @@ -35,8 +35,8 @@ This document provides answers to frequently asked questions (FAQ) when using AP In APISIX, a higher value indicates a higher route priority. * **Ingress:** Does not support explicit route priority. Routes created using Ingress are assigned a default priority of 0, typically the lowest. -* **HTTPRoute:** Has a [38-bit priority](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L428-L448). The priority calculation is dynamic and may change, making exact values difficult to predict. -* **APISIXRoute:** Can be assigned an explicit priority. To have a higher priority than an HTTPRoute, the value must exceed 549,755,813,887 (2^39 − 1). +* **HTTPRoute:** Uses priority bits 0 through 38 based on hostname length, match fields, and rule index. The [priority calculation](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L455-L524) is dynamic and may change, making exact values difficult to predict. Review Comment: Fixed after checking internal/adc/translator/httproute.go:455-524. The calculation starts at RuleIndexShiftBits = 7 and the highest occupied bit is 38, so I updated docs/en/latest/FAQ.md to say bits 7 through 38. ## docs/en/latest/FAQ.md: ## @@ -35,8 +35,8 @@ This document provides answers to frequently asked questions (FAQ) when using AP In APISIX, a higher value indicates a higher route priority. * **Ingress:** Does not support explicit route priority. Routes created using Ingress are assigned a default priority of 0, typically the lowest. -* **HTTPRoute:** Has a [38-bit priority](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L428-L448). The priority calculation is dynamic and may change, making exact values difficult to predict. -* **APISIXRoute:** Can be assigned an explicit priority. To have a higher priority than an HTTPRoute, the value must exceed 549,755,813,887 (2^39 − 1). +* **HTTPRoute:** Uses priority bits 0 through 38 based on hostname length, match fields, and rule index. The [priority calculation](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L455-L524) is dynamic and may change, making exact values difficult to predict. Review Comment: Fixed after checking the same calculation in internal/adc/translator/httproute.go:455-524. I updated docs/en/latest/FAQ.md to use a commit-pinned link to c48e9b3fb8d269b04b440c6fc5ed6880c05456f3 so the reference does not drift with master. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] docs: fix priority values and permalink in FAQ doc [apisix-ingress-controller]
kayx23 commented on PR #2712: URL: https://github.com/apache/apisix-ingress-controller/pull/2712#issuecomment-4448057316 Checked the two new Copilot review comments against . Both were valid: the FAQ now says HTTPRoute uses bits 7 through 38, and the source link is pinned to commit so it does not drift. The follow-up fix is in commit . I could not reply directly on the review threads from this account because GitHub returned a permission error for . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] docs: fix priority values and permalink in FAQ doc [apisix-ingress-controller]
Copilot commented on code in PR #2712: URL: https://github.com/apache/apisix-ingress-controller/pull/2712#discussion_r3239292426 ## docs/en/latest/FAQ.md: ## @@ -35,8 +35,8 @@ This document provides answers to frequently asked questions (FAQ) when using AP In APISIX, a higher value indicates a higher route priority. * **Ingress:** Does not support explicit route priority. Routes created using Ingress are assigned a default priority of 0, typically the lowest. -* **HTTPRoute:** Has a [38-bit priority](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L428-L448). The priority calculation is dynamic and may change, making exact values difficult to predict. -* **APISIXRoute:** Can be assigned an explicit priority. To have a higher priority than an HTTPRoute, the value must exceed 549,755,813,887 (2^39 − 1). +* **HTTPRoute:** Uses priority bits 0 through 38 based on hostname length, match fields, and rule index. The [priority calculation](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L455-L524) is dynamic and may change, making exact values difficult to predict. Review Comment: The text says HTTPRoute uses priority bits 0 through 38, but the implementation only sets fields starting at bit 7 (RuleIndexShiftBits=7) up to bit 38 (PreciseHostnameShiftBits=31 with an 8-bit length). Consider rewording to “uses bits 7–38 (up to bit 38)” to match the actual calculation. ## docs/en/latest/FAQ.md: ## @@ -35,8 +35,8 @@ This document provides answers to frequently asked questions (FAQ) when using AP In APISIX, a higher value indicates a higher route priority. * **Ingress:** Does not support explicit route priority. Routes created using Ingress are assigned a default priority of 0, typically the lowest. -* **HTTPRoute:** Has a [38-bit priority](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L428-L448). The priority calculation is dynamic and may change, making exact values difficult to predict. -* **APISIXRoute:** Can be assigned an explicit priority. To have a higher priority than an HTTPRoute, the value must exceed 549,755,813,887 (2^39 − 1). +* **HTTPRoute:** Uses priority bits 0 through 38 based on hostname length, match fields, and rule index. The [priority calculation](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L455-L524) is dynamic and may change, making exact values difficult to predict. Review Comment: The GitHub link is still branch-based (blob/master), so it can drift over time and isn’t a true permalink. If the intention is a stable reference, consider linking to a specific commit SHA (or tag) for the priority calculation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] docs: fix priority values and permalink in FAQ doc [apisix-ingress-controller]
Copilot commented on code in PR #2712: URL: https://github.com/apache/apisix-ingress-controller/pull/2712#discussion_r3238885311 ## docs/en/latest/FAQ.md: ## @@ -35,8 +35,8 @@ This document provides answers to frequently asked questions (FAQ) when using AP In APISIX, a higher value indicates a higher route priority. * **Ingress:** Does not support explicit route priority. Routes created using Ingress are assigned a default priority of 0, typically the lowest. -* **HTTPRoute:** Has a [38-bit priority](https://github.com/apache/apisix-ingress-controller/blob/master/internal/adc/translator/httproute.go#L428-L448). The priority calculation is dynamic and may change, making exact values difficult to predict. -* **APISIXRoute:** Can be assigned an explicit priority. To have a higher priority than an HTTPRoute, the value must exceed 549,755,813,887 (2^39 − 1). +* **HTTPRoute:** Has a [38-bit priority](https://github.com/apache/apisix-ingress-controller/blob/c48e9b3fb8d269b04b440c6fc5ed6880c05456f3/internal/adc/translator/httproute.go#L455-L472). The priority calculation is dynamic and may change, making exact values difficult to predict. +* **APISIXRoute:** Can be assigned an explicit priority. To have a higher priority than an HTTPRoute, the value must exceed 274,877,906,943 (2^38 − 1). Review Comment: The new threshold for APISIXRoute priority looks incorrect. `calculateHTTPRoutePriority` sets `maxNonWildcardLength << 31` with up to 8 bits of hostname length (bits 31–38), so HTTPRoute priority can exceed 2^38−1 (e.g., hostname length >= 128 already yields a value >= 2^38). If the intent is to document a safe value that always exceeds any HTTPRoute priority, the bound should be based on the highest used bit (bit 38), i.e. >= 2^39 (or > 2^39−1), and the text '38-bit priority' likely needs to be updated/clarified accordingly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] docs: fix priority values and permalink in FAQ doc [apisix-ingress-controller]
github-actions[bot] commented on PR #2712: URL: https://github.com/apache/apisix-ingress-controller/pull/2712#issuecomment-4384448712 This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] docs: fix priority values and permalink in FAQ doc [apisix-ingress-controller]
github-actions[bot] closed pull request #2712: docs: fix priority values and permalink in FAQ doc URL: https://github.com/apache/apisix-ingress-controller/pull/2712 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] docs: fix priority values and permalink in FAQ doc [apisix-ingress-controller]
github-actions[bot] commented on PR #2712: URL: https://github.com/apache/apisix-ingress-controller/pull/2712#issuecomment-4189924807 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
