You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Xinyu Liu <xi...@gmail.com> on 2020/02/13 19:19:22 UTC
Re: [DISCUSS] SEP-22: Container Placements in Samza
Thanks for writing a very detailed design doc. My feedback is below:
- The title of the SEP is Container Placements in Samza, which sounds like
it's going to reimplement the container placements logic currently in
ClusterBasedJobCoordinator. After reading the proposal, I think it should
be renamed to be a much smaller scope, e.g. Container Relocation Tool.
- Goal: the goal is to build a tool, right? I want to be crystal clear on
that. After the solution, I think there will be an tool to do container
relocation for Samza open source. Build the ability doesn't mean much to
Samza open source users I believe.
- Proposed Solution: after reading this I think the rejected the option is
to write to metadastore, and the accepted one is to develop a container
placement service. But then I read the architecture part, the proposal says
the preferred approach is to use Samza metastore API to read and write the
container placement metadata. It feels contradictory to me. Seems the
metadatsstore is a must-have piece in this solution, so it might be cleaner
to remove the first solution.
- I don't understand the "service" part of the container placement. Is it a
separate service that can be hosted somewhere? Is it based on jetty or
netty? what's the communication protocol? From reading the proposal, it
looks like a set of API instead of service. If that's the case, please
remove all the usage of "container placement service" in this proposal.
- key-value format: the description of the uuid of the
ContainerPlacementRequestMessage has a typo I think (response -> request).
- The diagram of the components interaction look nice, and definitely helps
me understand what the solution will look like. I didn't find a description
about writing the new locality information. I am interested in the ordering
of that regarding to the ordering of reserving resource and running the
container. I feel that part will be a bit complicated.
- public interfaces: are these interfaces intended to by used by samza
users? How do they use them? Is there an example? Are these interfaces
going into samza-api? Are those interfaces stable? Why the uuid in the
interface cannot be generated by Samza itself? Why
ContainerPlacementMetadatstore is part of the API? should that be hidden
from the users? If the goal is to provide a tool, I would rather make these
classes internal in samza-core and also describe how the tool will look
like here.
Thanks,
Xinyu
On Wed, Jan 15, 2020 at 10:34 AM Sanil Jain <sa...@gmail.com> wrote:
> Hi Prateek,
>
> Thanks for the feedback, I have updated the SEP with your suggestions.
> Please have a look.
>
> Thank you
> Sanil
>
> On Mon, 6 Jan 2020 at 10:54, Sanil Jain <sa...@gmail.com> wrote:
>
> > Hi all,
> >
> > Refreshing this back to see if there is any further feedback on this SEP.
> >
> > Thanks
> > Sanil
> >
> > On Thu, 5 Dec 2019 at 16:25, Sanil Jain <sa...@gmail.com> wrote:
> >
> >> Hi all,
> >>
> >> I have created SEP-22: Container Placements in Samza, which adds
> >> abilities to move containers for a Samza job seamlessly from one host to
> >> another without restarting jobs.
> >>
> >> Please find the SEP wiki below:
> >>
> https://cwiki.apache.org/confluence/display/SAMZA/SEP-22%3A+Container+Placements+in+Samza
> >>
> >> Please take a look and chime in your feedback.
> >>
> >> Thanks
> >> Sanil
> >>
> >
>
Re: [DISCUSS] SEP-22: Container Placements in Samza
Posted by Sanil Jain <sa...@gmail.com>.
Hi Xinyu,
Apologies, looks like I sent this from my Li email and was not updated, let
me send it again from my personal email.
Thanks for the feedback, here are my comments:
The title of the SEP is Container Placements in Samza, which sounds like
> it's going to reimplement the container placements logic currently in
> ClusterBasedJobCoordinator. After reading the proposal, I think it should
> be renamed to be a much smaller scope, e.g. Container Relocation Tool.
Container Placements title suggests building the ability to move/restart
containers of a job on a same or different host, the external tool moving
containers is one user of it, other users can be a Load balancer controller
(Cruise control like system), ASC controller resizing containers with
different sizes
Some other abilities are:
1. It will also give as an ability to override already existing locality
when one or partial containers are down with job running in a degraded state
2. Spin up & place a standby for one or few containers
Hence I feel the naming is justified
- Goal: the goal is to build a tool, right? I want to be crystal clear on
> that. After the solution, I think there will be an tool to do container
> relocation for Samza open source. Build the ability doesn't mean much to
> Samza open source users I believe.
Sure the Goals section mention "Expose these APIs via a tool or dashboard
for operability", I can add more emphasis on it. At the same time,
open-source users can use these API in their custom tools for addressing
Operability
- Proposed Solution: after reading this I think the rejected the option is
> to write to metadastore, and the accepted one is to develop a container
> placement service. But then I read the architecture part, the proposal
> says
> the preferred approach is to use Samza metastore API to read and write the
> container placement metadata. It feels contradictory to me. Seems the
> metadatsstore is a must-have piece in this solution, so it might be cleaner
> to remove the first solution.
Nice catch, the first approach is actually directly modifying the locality
mapping in the metadata store (coordinator stream) as compared to the other
approach where we use metadata store to read/write Container placement
request/response under a *separate metastore namespace. *Let me add these
details so others don't get confused
- I don't understand the "service" part of the container placement. Is it a
> separate service that can be hosted somewhere? Is it based on jetty or
> netty? what's the communication protocol? From reading the proposal, it
> looks like a set of API instead of service. If that's the case, please
> remove all the usage of "container placement service" in this proposal.
Its referred to as a service since there is a listener of events running in
the JobCoordinator (ContainerPlacementHandler) which relays the container
placement request messages to the JobCoordinator, metastore is just used as
a communication channel which we can logically compare with a REST Service
which where we have an event handler listening on a port for request. The
communication protocol for request/response messages is defined by us using
metastore (Kafka) as a communication channel. It's not an independent
service that can be hosted somewhere but more an embedded one.
If can think of a better alternative for this & follow up.
- key-value format: the description of the uuid of the
> ContainerPlacementRequestMessage has a typo I think (response -> request).
Corrected, thanks!
- The diagram of the components interaction look nice, and definitely helps
> me understand what the solution will look like. I didn't find a description
> about writing the new locality information. I am interested in the ordering
> of that regarding to the ordering of reserving resource and running the
> container. I feel that part will be a bit complicated.
Nice catch, I added this section to the doc:
*Who writes the new locality mapping after a successful move?*
Samza container on a successful start write their new locality message to
the metadata store (code
<https://github.com/apache/samza/blob/master/samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala#L749>),
hence after a successful move container writes its new locality
- public interfaces: are these interfaces intended to by used by samza
> users? How do they use them? Is there an example? Are these interfaces
> going into samza-api? Are those interfaces stable? Why the uuid in the
> interface cannot be generated by Samza itself? Why
Let me add an example to use them and add two-section Public Interfaces
containing (ContainerPlacementMessage, ContainerPlacementRequestMessage,
and ContainerPlacementResponseMessage) with the interface stability
evolving. UUID is generated by samza itself, please check
ContainerPlacementMetadataStore#writeContainerPlacementRequestMessage. In
terms of usage please see the answer to the next question I added a sample
on SEP showing use of these interfaces:
> ContainerPlacementMetadatstore is part of the API? should that be hidden
> from the users? If the goal is to provide a tool, I would rather make these
> classes internal in samza-core and also describe how the tool will look
> like here.
ContainerPlacementMetadatstore is already part of samza core and I added
another section giving a sample usage:
*Example Usage:*
place-container --deployment-id 1581635852024-5117e303 --app-name snjain-
test-cp --app.id = 1 --processor-id 4 --request-expiry 10
--destination-host abc.prod.com
*Example Tool:*
@CommandLine.Command(name = "place-container", description = "Request to
move/restart container at destination-host")
public class ContainerPlacementTool {
...
_appName = // read from commandline
_appId = // read from commandline
_deploymentId = // read from commandline
_processorId = // read from commandline
_destinationHost = // read from commandline
_requestExpiry = // read from commandline
MetadataStore metadataStore = buildMetadataStore(_appName, _appId);
try {
ContainerPlacementMetadataStore containerPlacementMetadataStore =
new ContainerPlacementMetadataStore(metadataStore);
containerPlacementMetadataStore.start();
Duration requestExpiry = _requestExpiry != null ?
Duration.ofSeconds(_requestExpiry) : null;
UUID uuid =
containerPlacementMetadataStore.writeContainerPlacementRequestMessage(_deploymentId,
_processorId,
_destinationHost, _requestExpiry, System.currentTimeMillis());
System.out.println("Request received query the status using: " +
uuid);
} finally {
metadataStore.close();
}
}
Thanks
Sanil
On Thu, 13 Feb 2020 at 11:19, Xinyu Liu <xi...@gmail.com> wrote:
> Thanks for writing a very detailed design doc. My feedback is below:
>
> - The title of the SEP is Container Placements in Samza, which sounds like
> it's going to reimplement the container placements logic currently in
> ClusterBasedJobCoordinator. After reading the proposal, I think it should
> be renamed to be a much smaller scope, e.g. Container Relocation Tool.
> - Goal: the goal is to build a tool, right? I want to be crystal clear on
> that. After the solution, I think there will be an tool to do container
> relocation for Samza open source. Build the ability doesn't mean much to
> Samza open source users I believe.
> - Proposed Solution: after reading this I think the rejected the option is
> to write to metadastore, and the accepted one is to develop a container
> placement service. But then I read the architecture part, the proposal says
> the preferred approach is to use Samza metastore API to read and write the
> container placement metadata. It feels contradictory to me. Seems the
> metadatsstore is a must-have piece in this solution, so it might be cleaner
> to remove the first solution.
> - I don't understand the "service" part of the container placement. Is it a
> separate service that can be hosted somewhere? Is it based on jetty or
> netty? what's the communication protocol? From reading the proposal, it
> looks like a set of API instead of service. If that's the case, please
> remove all the usage of "container placement service" in this proposal.
> - key-value format: the description of the uuid of the
> ContainerPlacementRequestMessage has a typo I think (response -> request).
> - The diagram of the components interaction look nice, and definitely helps
> me understand what the solution will look like. I didn't find a description
> about writing the new locality information. I am interested in the ordering
> of that regarding to the ordering of reserving resource and running the
> container. I feel that part will be a bit complicated.
> - public interfaces: are these interfaces intended to by used by samza
> users? How do they use them? Is there an example? Are these interfaces
> going into samza-api? Are those interfaces stable? Why the uuid in the
> interface cannot be generated by Samza itself? Why
> ContainerPlacementMetadatstore is part of the API? should that be hidden
> from the users? If the goal is to provide a tool, I would rather make these
> classes internal in samza-core and also describe how the tool will look
> like here.
>
> Thanks,
> Xinyu
>
>
>
> On Wed, Jan 15, 2020 at 10:34 AM Sanil Jain <sa...@gmail.com>
> wrote:
>
> > Hi Prateek,
> >
> > Thanks for the feedback, I have updated the SEP with your suggestions.
> > Please have a look.
> >
> > Thank you
> > Sanil
> >
> > On Mon, 6 Jan 2020 at 10:54, Sanil Jain <sa...@gmail.com> wrote:
> >
> > > Hi all,
> > >
> > > Refreshing this back to see if there is any further feedback on this
> SEP.
> > >
> > > Thanks
> > > Sanil
> > >
> > > On Thu, 5 Dec 2019 at 16:25, Sanil Jain <sa...@gmail.com>
> wrote:
> > >
> > >> Hi all,
> > >>
> > >> I have created SEP-22: Container Placements in Samza, which adds
> > >> abilities to move containers for a Samza job seamlessly from one host
> to
> > >> another without restarting jobs.
> > >>
> > >> Please find the SEP wiki below:
> > >>
> >
> https://cwiki.apache.org/confluence/display/SAMZA/SEP-22%3A+Container+Placements+in+Samza
> > >>
> > >> Please take a look and chime in your feedback.
> > >>
> > >> Thanks
> > >> Sanil
> > >>
> > >
> >
>