[jira] [Commented] (YARN-5709) Cleanup leader election related configuration mess
[
https://issues.apache.org/jira/browse/YARN-5709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15727438#comment-15727438
]
Karthik Kambatla commented on YARN-5709:
The patch posted here captures my intention. [~jianhe] - what do you think?
> Cleanup leader election related configuration mess
> --
>
> Key: YARN-5709
> URL: https://issues.apache.org/jira/browse/YARN-5709
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: resourcemanager
>Affects Versions: 2.8.0
>Reporter: Karthik Kambatla
>Assignee: Karthik Kambatla
>Priority: Blocker
> Attachments: yarn-5709.1.patch
>
>
> While reviewing YARN-5677 and YARN-5694, I noticed we could make the
> curator-based election code cleaner. It is nicer to get this fixed in 2.8
> before we ship it, but this can be done at a later time as well.
> # By EmbeddedElector, we meant it was running as part of the RM daemon. Since
> the Curator-based elector is also running embedded, I feel the code should be
> checking for {{!curatorBased}} instead of {{isEmbeddedElector}}
> # {{LeaderElectorService}} should probably be named
> {{CuratorBasedEmbeddedElectorService}} or some such.
> # The code that initializes the elector should be at the same place
> irrespective of whether it is curator-based or not.
> # We seem to be caching the CuratorFramework instance in RM. It makes more
> sense for it to be in RMContext. If others are okay with it, we might even be
> better of having {{RMContext#getCurator()}} method to lazily create the
> curator framework and then cache it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5709) Cleanup leader election related configuration mess
[
https://issues.apache.org/jira/browse/YARN-5709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15726972#comment-15726972
]
Jian He commented on YARN-5709:
---
yeah... I mean.. if we already have an explicit config
(curator-based-elector-enabled), then we don't require a separate
{{yarn.resourcemanager.ha.automatic-failover.embedded}} config, as
curator-based-elector-enabled implied it is embedded.
> Cleanup leader election related configuration mess
> --
>
> Key: YARN-5709
> URL: https://issues.apache.org/jira/browse/YARN-5709
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: resourcemanager
>Affects Versions: 2.8.0
>Reporter: Karthik Kambatla
>Assignee: Daniel Templeton
>Priority: Blocker
>
> While reviewing YARN-5677 and YARN-5694, I noticed we could make the
> curator-based election code cleaner. It is nicer to get this fixed in 2.8
> before we ship it, but this can be done at a later time as well.
> # By EmbeddedElector, we meant it was running as part of the RM daemon. Since
> the Curator-based elector is also running embedded, I feel the code should be
> checking for {{!curatorBased}} instead of {{isEmbeddedElector}}
> # {{LeaderElectorService}} should probably be named
> {{CuratorBasedEmbeddedElectorService}} or some such.
> # The code that initializes the elector should be at the same place
> irrespective of whether it is curator-based or not.
> # We seem to be caching the CuratorFramework instance in RM. It makes more
> sense for it to be in RMContext. If others are okay with it, we might even be
> better of having {{RMContext#getCurator()}} method to lazily create the
> curator framework and then cache it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5709) Cleanup leader election related configuration mess
[
https://issues.apache.org/jira/browse/YARN-5709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15726943#comment-15726943
]
Karthik Kambatla commented on YARN-5709:
bq. For the original hadoop-common's elector, I agree we should keep using this
config, otherwise, it's incompatible.
{{yarn.resourcemanager.ha.automatic-failover.embedded}} only says the leader
election is embedded. It does not say anything about the actual implementation
of it. It just happened to be ActiveStandbyElector. Now, we are replacing it
with the curator-based one. No?
> Cleanup leader election related configuration mess
> --
>
> Key: YARN-5709
> URL: https://issues.apache.org/jira/browse/YARN-5709
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: resourcemanager
>Affects Versions: 2.8.0
>Reporter: Karthik Kambatla
>Assignee: Daniel Templeton
>Priority: Blocker
>
> While reviewing YARN-5677 and YARN-5694, I noticed we could make the
> curator-based election code cleaner. It is nicer to get this fixed in 2.8
> before we ship it, but this can be done at a later time as well.
> # By EmbeddedElector, we meant it was running as part of the RM daemon. Since
> the Curator-based elector is also running embedded, I feel the code should be
> checking for {{!curatorBased}} instead of {{isEmbeddedElector}}
> # {{LeaderElectorService}} should probably be named
> {{CuratorBasedEmbeddedElectorService}} or some such.
> # The code that initializes the elector should be at the same place
> irrespective of whether it is curator-based or not.
> # We seem to be caching the CuratorFramework instance in RM. It makes more
> sense for it to be in RMContext. If others are okay with it, we might even be
> better of having {{RMContext#getCurator()}} method to lazily create the
> curator framework and then cache it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5709) Cleanup leader election related configuration mess
[
https://issues.apache.org/jira/browse/YARN-5709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15726920#comment-15726920
]
Jian He commented on YARN-5709:
---
bq. yarn.resourcemanager.ha.automatic-failover.embedded
For the original hadoop-common's elector, I agree we should keep using this
config, otherwise, it's incompatible.
But for the curator-based implementation. I feel we can avoid this config. One
less config is one step easier to config HA. This config looks like a
placeholder for future implementation only, which can never be set to false as
of now. IMHO, instead, a ZK-FC-based-elector config can be added later if it's
really implemented in the future.
bq. Any reason we are not replacing the implementation? Do we just want to be
safe and have a workaround in case the curator-based elector turns out to
broken?
Yeah, just intends to be safe.
bq. If that is the case, the config that determines the implementation should
be removed in a subsequent release and accordingly be called out @Unstable. We
should likely not list it in yarn-default.xml.
agree
bq. If we do decide on moving the initialization, we should move it for both
implementations and not just one.
Agree we can move both.
bq. However, if we were to implement a ZKFC-based elector, that would have to
depend on the AdminService to affect any transitions at all. I believe Bikas
has recommended we keep the same code path irrespective of whether leader
election is embedded. I see merit to that argument.
Didn't actually get the argument about keeping the same code path... If
EmbeddedElectorService is moved outside of AdminService, it still uses the
same code path. EmbeddedElectorService -> AdminService ->
transitionToActive/Standby. For ZKFC-based elector, it's the same. IIUC,
whether EmbeddedElectorService is initialized inside AdminService or outside
does not affect reusing the same code path. And conceptually
EmbeddedElectorService does not look like a sub-service of AdminService..
> Cleanup leader election related configuration mess
> --
>
> Key: YARN-5709
> URL: https://issues.apache.org/jira/browse/YARN-5709
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: resourcemanager
>Affects Versions: 2.8.0
>Reporter: Karthik Kambatla
>Assignee: Daniel Templeton
>Priority: Blocker
>
> While reviewing YARN-5677 and YARN-5694, I noticed we could make the
> curator-based election code cleaner. It is nicer to get this fixed in 2.8
> before we ship it, but this can be done at a later time as well.
> # By EmbeddedElector, we meant it was running as part of the RM daemon. Since
> the Curator-based elector is also running embedded, I feel the code should be
> checking for {{!curatorBased}} instead of {{isEmbeddedElector}}
> # {{LeaderElectorService}} should probably be named
> {{CuratorBasedEmbeddedElectorService}} or some such.
> # The code that initializes the elector should be at the same place
> irrespective of whether it is curator-based or not.
> # We seem to be caching the CuratorFramework instance in RM. It makes more
> sense for it to be in RMContext. If others are okay with it, we might even be
> better of having {{RMContext#getCurator()}} method to lazily create the
> curator framework and then cache it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5709) Cleanup leader election related configuration mess
[
https://issues.apache.org/jira/browse/YARN-5709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15726506#comment-15726506
]
Karthik Kambatla commented on YARN-5709:
My primary concern with the current 2.8.0 code is confusing configs. The reason
we called it existing leader election and the corresponding config embedded was
because we wanted to highlight it is embedded in the RM. The plan, at the time,
was to add ZKFC-based leader election as well. We should likely leave that
config ({{yarn.resourcemanager.ha.automatic-failover.embedded}}) alone unless
we make a decision that we will not add ZKFC-type of leader election that runs
in a different process.
bq. the ultimate goal is to remove the old EmbeddedElectorService
We are in agreement here. I am comfortable with ripping out the current
implementation of EmbeddedElectorService in 2.8.0 and replacing it with the
curator-based implementation.
Any reason we are not replacing the implementation? Do we just want to be safe
and have a workaround in case the curator-based elector turns out to broken? If
that is the case, the config that determines the implementation should be
removed in a subsequent release and accordingly be called out @Unstable. We
should likely not list it in yarn-default.xml.
bq. don't think it needs to be the case even for the old
EmbeddedElectorService too, if you look at the implementation, there's no
dependency between the EmbeddedElectorService and AdminService at all.
I see your point.
On the dependency front, EmbeddedElectorService does not depend on
AdminService. However, if we were to implement a ZKFC-based elector, that would
have to depend on the AdminService to affect any transitions at all. I believe
Bikas has recommended we keep the same code path irrespective of whether leader
election is embedded. I see merit to that argument.
If we do decide on moving the initialization, we should move it for both
implementations and not just one. Implementation-based initialization points is
confusing for any new person looking at the code. Even for those looking at it
after a while (like me).
> Cleanup leader election related configuration mess
> --
>
> Key: YARN-5709
> URL: https://issues.apache.org/jira/browse/YARN-5709
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: resourcemanager
>Affects Versions: 2.8.0
>Reporter: Karthik Kambatla
>Assignee: Daniel Templeton
>Priority: Blocker
>
> While reviewing YARN-5677 and YARN-5694, I noticed we could make the
> curator-based election code cleaner. It is nicer to get this fixed in 2.8
> before we ship it, but this can be done at a later time as well.
> # By EmbeddedElector, we meant it was running as part of the RM daemon. Since
> the Curator-based elector is also running embedded, I feel the code should be
> checking for {{!curatorBased}} instead of {{isEmbeddedElector}}
> # {{LeaderElectorService}} should probably be named
> {{CuratorBasedEmbeddedElectorService}} or some such.
> # The code that initializes the elector should be at the same place
> irrespective of whether it is curator-based or not.
> # We seem to be caching the CuratorFramework instance in RM. It makes more
> sense for it to be in RMContext. If others are okay with it, we might even be
> better of having {{RMContext#getCurator()}} method to lazily create the
> curator framework and then cache it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
