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

[improve][pip] PIP-380: Support setting up specific namespaces to skipping the load-shedding #23304

Conversation

Demogorgon314
Copy link
Member

No description provided.

@Demogorgon314 Demogorgon314 added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. type/PIP ready-to-test PIP labels Sep 13, 2024
@Demogorgon314 Demogorgon314 self-assigned this Sep 13, 2024
@lhotari
Copy link
Member

lhotari commented Sep 13, 2024

It seems fine. Just thinking that there could be similar requirements for user defined namespaces and therefore having a property in the namespace policy to control this would be useful. Instead of a single binary value, perhaps it could be some type of value that would be used for ordering the namespace bundles that are picked as candidates for load shedding.
A system namespace could use a value that would result in all other namespace bundles being picked before it becomes a candidate for load shedding.

@heesung-sn
Copy link
Contributor

heesung-sn commented Sep 14, 2024

I wonder if we can limit this blacklisting to the TransferShedder under loadBalanceTransferShedderExcludedNamespaces config .

@Demogorgon314
Copy link
Member Author

@heesung-sn I would like to use the loadBalancerSheddingExcludedNamespaces config to support ExtensibleLoadManager and ModularLoadManager

@heesung-sn
Copy link
Contributor

I think we probably want to explore namespace policy like this work.

#23120

@Demogorgon314
Copy link
Member Author

Adding a namespace policy is a good idea, let me update the PIP.

@Demogorgon314 Demogorgon314 marked this pull request as draft September 14, 2024 04:50
@Demogorgon314
Copy link
Member Author

@lhotari Are you suggesting adding a loadSheddingPriority to the namespace police? I'm wondering which case will use the priority for load shedding. And when using priority for each namespace will make the load-shedding strategy let bit complex, since when we select the hot bundle to unload we have to consider the namespace priority. I prefer to use loadSheddingEnabled, then for the protocol's system topic and some user's benchmark topic we can make the ownership stable.

@Demogorgon314
Copy link
Member Author

ping @lhotari

pip/pip_379.md Outdated Show resolved Hide resolved
@lhotari lhotari changed the title [improve][pip] PIP-379: Support setting up specific namespaces to skipping the load-shedding [improve][pip] PIP-380: Support setting up specific namespaces to skipping the load-shedding Sep 18, 2024
@lhotari
Copy link
Member

lhotari commented Sep 18, 2024

PIP-379 is already taken, I increased the number to PIP-380. The way to find the next available number is to check the dev mailing list and open PRs. The first one to open a discussion for a specific PIP number takes it. This is the PIP-379 discussion: https://lists.apache.org/thread/l5zjq0fb2dscys3rsn6kfl7505tbndlx . I somehow missed your PR when I made the PIP number assignment. I'm sorry about that.

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lhotari
Copy link
Member

lhotari commented Sep 18, 2024

@lhotari Are you suggesting adding a loadSheddingPriority to the namespace police? I'm wondering which case will use the priority for load shedding. And when using priority for each namespace will make the load-shedding strategy let bit complex, since when we select the hot bundle to unload we have to consider the namespace priority. I prefer to use loadSheddingEnabled, then for the protocol's system topic and some user's benchmark topic we can make the ownership stable.

@Demogorgon314 It was something to consider for the future.

@Demogorgon314 Demogorgon314 marked this pull request as ready for review September 26, 2024 07:02
@lhotari
Copy link
Member

lhotari commented Oct 9, 2024

@Demogorgon314 Please update the links to the PIP discussion and voting threads.

@Demogorgon314 Demogorgon314 merged commit b5484f6 into apache:master Nov 8, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. PIP ready-to-test type/PIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants