You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2022/04/08 13:46:15 UTC

[GitHub] [helix] hmhagberg opened a new issue, #2024: Are all resources expected to have ResourceConfig?

hmhagberg opened a new issue, #2024:
URL: https://github.com/apache/helix/issues/2024

   [`ConfigAccessor#getResourceConfig`](https://github.com/apache/helix/blob/2d9fd3cc525edb251b6e41904bccfd78654947e7/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java#L835) logs warning (and returns `null`) if config for some resource is missing from ZooKeeper. We are seeing quite a few warnings about missing resource configs due to this. However, the system appears to be working correctly regardless. Example warning:
   
   ```
   2022-04-08 10:45:38,563 +0200 WARN  [ConfigAccessor           ] - No config found at /foo-cluster/CONFIGS/RESOURCE/bar-resource
   ```
   
   Are all resources expected to have `ResourceConfig`? If not, the warning appears to be invalid and should be removed or lowered to info. If yes, it would be great if either an empty `ResourceConfig` was automatically created or requirement to set `ResourceConfig` was reflected in documentation and `HelixAdmin` API.


-- 
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: reviews-unsubscribe@helix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] narendly commented on issue #2024: Are all resources expected to have ResourceConfig?

Posted by GitBox <gi...@apache.org>.
narendly commented on issue #2024:
URL: https://github.com/apache/helix/issues/2024#issuecomment-1092988914

   @hmhagberg Thanks for explaining where the warn log was coming from.
   
   > Good info! So the warning is indeed valid and we should be creating ResourceConfig along with IdealState.
   Actually, that's not what I meant - that is only so if the application needs to encode more configs (other than what's required in IdealState). In such cases, a ResourceConfig must be created and the application should add the configs in ResourceConfig, not IdealState.
   
   If there aren't such extra configs, then ResourceConfig is optional.
   
   But in your case though, it seems that you are using dedicated thread pools for your resources? If so, are you creating a proper ResourceConfig and adding a thread pool size field to it?


-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] narendly commented on issue #2024: Are all resources expected to have ResourceConfig?

Posted by GitBox <gi...@apache.org>.
narendly commented on issue #2024:
URL: https://github.com/apache/helix/issues/2024#issuecomment-1095317395

   @hmhagberg we can have someone look further into this issue. If it is indeed the case that we're unnecessarily emitting logs then that'd need fixing.


-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


Re: [I] Are all resources expected to have ResourceConfig? [helix]

Posted by "junkaixue (via GitHub)" <gi...@apache.org>.
junkaixue closed issue #2024: Are all resources expected to have ResourceConfig?
URL: https://github.com/apache/helix/issues/2024


-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] hmhagberg commented on issue #2024: Are all resources expected to have ResourceConfig?

Posted by GitBox <gi...@apache.org>.
hmhagberg commented on issue #2024:
URL: https://github.com/apache/helix/issues/2024#issuecomment-1094830018

   @narendly alright, in that case we should be good with just IdealState since we aren't actually using dedicated thread pools. My understanding of the ` updateStateTransitionMessageThreadPool` method is that its purpose is to check if dedicated thread pool has been configured for a resource. Hence it's a bit confusing to get a warning since our intention is not to use one.
   
   You can quite easily reproduce the warning by running [TestRestartParticipant](https://github.com/apache/helix/blob/master/helix-core/src/test/java/org/apache/helix/integration/paticipant/TestRestartParticipant.java#L57) test (you need to change level of [`org.apache`](https://github.com/apache/helix/blob/master/helix-core/src/test/resources/log4j2.properties#L53) logger first).
   
   ```
   69335 [ClusterManager_Watcher_TestRestartParticipant_localhost_12918_PARTICIPANT_10001] WARN  org.apache.helix.ConfigAccessor [] - No config found at /TestRestartParticipant/CONFIGS/RESOURCE/TestDB0.
   ```


-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] narendly commented on issue #2024: Are all resources expected to have ResourceConfig?

Posted by GitBox <gi...@apache.org>.
narendly commented on issue #2024:
URL: https://github.com/apache/helix/issues/2024#issuecomment-1092885415

   Hi @hmhagberg - thank you for the question. The historical context is this: IdealState is the de-facto definition for a resource with its required fields, but applications wanted to add more application-specific configs. But the original intention was to make IdealStates immutable (not enforced, unfortunately), so Helix added support for ResourceConfig. This is why things can work without ResourceConfig, but the intention is for a given resource to have a ResourceConfig and an IdealState pair.
   
   But the log message seems to be coming from ConfigAccessor, which is entirely used by the application logic. Are you trying to ConfigAccessor#get? This, I don't believe, is being called by the Helix controller pipeline.


-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


Re: [I] Are all resources expected to have ResourceConfig? [helix]

Posted by "junkaixue (via GitHub)" <gi...@apache.org>.
junkaixue commented on issue #2024:
URL: https://github.com/apache/helix/issues/2024#issuecomment-2711944886

   Feel free to reopen if necessary.


-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] hmhagberg commented on issue #2024: Are all resources expected to have ResourceConfig?

Posted by GitBox <gi...@apache.org>.
hmhagberg commented on issue #2024:
URL: https://github.com/apache/helix/issues/2024#issuecomment-1092963611

   @narendly thanks for the quick reply! We are not using ConfigAccessor in our application logic. However, I was wrong about the warning coming from `getResourceConfig` -- instead it comes from `get` which is called with scope RESOURCE in HelixTaskExecutor [here](https://github.com/apache/helix/blob/master/helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskExecutor.java#L318).
   
   >the intention is for a given resource to have a ResourceConfig and an IdealState pair
   
   Good info! So the warning is indeed valid and we should be creating ResourceConfig along with IdealState.
   
   Do you think it would make sense to create ResourceConfig by default in `HelixAdmin#addResource`?


-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org