Skip to content

[#7754] improvement(storage) : Add a configuration to disable entity store GC for RelationalEntityStore#7777

Closed
georgereuben wants to merge 6 commits intoapache:mainfrom
georgereuben:feat/disable-entity-store-gc
Closed

[#7754] improvement(storage) : Add a configuration to disable entity store GC for RelationalEntityStore#7777
georgereuben wants to merge 6 commits intoapache:mainfrom
georgereuben:feat/disable-entity-store-gc

Conversation

@georgereuben
Copy link
Contributor

What changes were proposed in this pull request?

Added a configuration to disable GC for relational entity stores

Why are the changes needed?

To provide an option to disable entity store GC in some server to make only one server start the GC in case multiple servers use the same RDMS.

Fix: #7754

Does this PR introduce any user-facing change?

No

How was this patch tested?

N/A

@yuqi1129
Copy link
Contributor

@FANNG1
Have you reached an agreement that a switch was necessary to address this issue?

@FANNG1
Copy link
Contributor

FANNG1 commented Jul 22, 2025

@FANNG1 Have you reached an agreement that a switch was necessary to address this issue?

Not seeing opponent options, do you have a better suggestion?

@yuqi1129
Copy link
Contributor

@FANNG1 Have you reached an agreement that a switch was necessary to address this issue?

Not seeing opponent options, do you have a better suggestion?

No, other components like statistic collectors also face the same problem, so I wonder if there is a unified solution.

@FANNG1
Copy link
Contributor

FANNG1 commented Jul 23, 2025

@FANNG1 Have you reached an agreement that a switch was necessary to address this issue?

Not seeing opponent options, do you have a better suggestion?

No, other components like statistic collectors also face the same problem, so I wonder if there is a unified solution.

This may need a long-term, complicated solution, this could be a short-term solution to avoid unexpected results and errrors.

@jerryshao
Copy link
Contributor

What's the problem here? Will another GC lead to a wrong result?

@FANNG1
Copy link
Contributor

FANNG1 commented Jul 23, 2025

What's the problem here? Will another GC lead to a wrong result?

I'm not sure whether there will be error results, but I see many error & warning logs

@jerryshao
Copy link
Contributor

jerryshao commented Jul 23, 2025

I agree with @yuqi1129; adding a configuration is easy. However, we should have a thorough solution for multi-server deployment, not a band-aid fix. At least we should identify the errors and figure out reasons.

We haven't claimed that we support multi-active deployment, such problem should be treated as a whole to address this problem.

@georgereuben georgereuben marked this pull request as draft July 23, 2025 09:54
@georgereuben
Copy link
Contributor Author

Hi @yuqi1129 @FANNG1 , just wanted to confirm if I should continue working on this issue or are we looking for long term solutions? Please let me know

@FANNG1
Copy link
Contributor

FANNG1 commented Jul 28, 2025

Hi @yuqi1129 @FANNG1 , just wanted to confirm if I should continue working on this issue or are we looking for long term solutions? Please let me know

We could wait for long-term solutions.

@jerryshao jerryshao closed this Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Add a configuration to disable entity store gc

4 participants