ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexei Scherbakov <alexey.scherbak...@gmail.com>
Subject Re: Terms clarification and modules splitting logic
Date Thu, 08 Apr 2021 13:59:25 GMT
Alexey,

Thanks for the detailed explanation.

Ok, let's agree on having the internal package. I've created the ticket [1]
to unify it's usage within the project.

[1] https://issues.apache.org/jira/browse/IGNITE-14506

чт, 8 апр. 2021 г. в 15:34, Alexey Goncharuk <alexey.goncharuk@gmail.com>:

> Alexei,
>
> The main benefit from Jigsaw that I see for the project structure is
> controllable module interaction.
>
> Let's take our networking module as an example first. We may want to make
> sure that module implementation specifics do not leak to outside modules,
> so we define in the module definition that the module exports package
> org.apache.ignite.internal.network. Now, even if we have a public class in
> package org.apache.ignite.internal.network.scalecube (the class may be
> public for many reasons, including a need for access in other
> implementation packages), other modules will not be able to directly work
> with this public class - the code will not compile.
>
> Another important feature of Jigsaw is the qualified export statement that
> limits the exported API to specific modules. Let's say for some reason we
> want to limit Raft client usage only to metastorage and partition
> components. Then we can specify in the raft module descriptor that raft API
> is only exported to metastorage and partition modules. Other modules will
> not compile if they will try to work with raft API.
>
> To me, this looks like a very powerful mechanism allowing to strictly
> define modules structure and hierarchy.
>
> As for the utility classes, @Internal looks less obvious for me because a
> user cannot directly see it without looking at the class itself. When
> 'internal' is imprinted in the package, you can see the violation directly
> at the usage site because there will be an import statement with an
> 'internal' package. You can check this as simple as an obvious grep
> command, which will not work with the annotation.
>
> --AG
>
> ср, 31 мар. 2021 г. в 21:04, Alexei Scherbakov <
> alexey.scherbakoff@gmail.com
> >:
>
> > Alexey,
> >
> > Can you provide us some details on jygsaw adoption to better understand
> > the benefits ?
> >
> > "We should be free to change them without any compatibility contract" -
> > let's mark such classes with a special annotation like @Internal, will it
> > work for you ?
> >
> >
> >
> > ср, 31 мар. 2021 г. в 15:10, Alexey Goncharuk <
> alexey.goncharuk@gmail.com
> > >:
> >
> > > This won't work with the Java Jigsaw module system because it prohibits
> > > having two identical packages in different modules. I really hope that
> we
> > > will adopt Jigsaw in the near future. Unless you are suggesting moving
> > all
> > > utility classes under org.apache.ignite.api.util package, bit this
> looks
> > > really odd to me - why would IgniteUuid be in api.util package?
> > >
> > > As for the public and private utilities, I think there may be some
> > classes
> > > that may be common for all modules, but should not be treated as public
> > API
> > > because we should be free to change them without any compatibility
> > > contract. An example of such a class is GridFunc. Arguably, many of its
> > > methods should be removed for good, but I am sure there will be a few
> > > really useful ones. Nevertheless, we should not encourage or allow
> users
> > to
> > > use GridFunc.
> > >
> > > --AG
> > >
> > > ср, 31 мар. 2021 г. в 14:27, Alexei Scherbakov <
> > > alexey.scherbakoff@gmail.com
> > > >:
> > >
> > > > Alexey,
> > > >
> > > > I would instead  suggest moving the public utility classes to
> > > > org.apache.ignite.api. package in the util module to separate them
> from
> > > > internal classes, if we really need this.
> > > >
> > > > Actually, I don't think there is a point in separating
> public/internal
> > > > classes in the util module. What are the benefits of this ?
> > > >
> > > > ср, 31 мар. 2021 г. в 12:16, Alexey Goncharuk <
> > > alexey.goncharuk@gmail.com
> > > > >:
> > > >
> > > > > Alexei,
> > > > >
> > > > > I had the same opinion regarding the internal package, but we still
> > > need
> > > > to
> > > > > somehow distinguish between public and internal classes in the
> > > > ignite-util
> > > > > module. If we introduce the internal package in the util, we should
> > > > follow
> > > > > the same structure in other modules.
> > > > >
> > > > > Thoughts?
> > > > >
> > > > > вт, 30 мар. 2021 г. в 18:37, Alexei Scherbakov <
> > > > > alexey.scherbakoff@gmail.com
> > > > > >:
> > > > >
> > > > > > +1 to package and module naming.
> > > > > > +1 to service definition as "component providing a high-level
API
> > to
> > > > > > user/other components/services"
> > > > > >
> > > > > > I would avoid defining strict rules for Manager and Processor.
> > > > > > For me it just adds confusion without real value.
> > > > > > A component can be a Manager if it manages something, a Processor
> > if
> > > it
> > > > > > processes something, and so on.
> > > > > > I think having Component and Service (which is also a Component)
> is
> > > > > enough.
> > > > > > Any component can be singleton or not - it's defined by its
> > > lifecycle.
> > > > > >
> > > > > > +1 to renaming core to something more meaningful, but the name
> lang
> > > > > doesn't
> > > > > > fit for a collection of utility classes for me, I would prefer
> > > > > ignite-util.
> > > > > > Apache Tomcat has the same jar, for reference. I'm also fine
to
> > leave
> > > > it
> > > > > as
> > > > > > is.
> > > > > > -1 to have an "internal" package. All modules are known to be
> > > internal
> > > > > > except api and (partially) util, so why bother at all?
> > > > > >
> > > > > >
> > > > > > вт, 30 мар. 2021 г. в 12:05, Andrey Mashenkov <
> > > > > andrey.mashenkov@gmail.com
> > > > > > >:
> > > > > >
> > > > > > > Agree with package and module naming.
> > > > > > >
> > > > > > > I just thought that
> > > > > > > Service is a self-suffucient component and provides high-level
> > API
> > > to
> > > > > > > user/other components/services (e.g. RaftService to
> > TableService).
> > > > > > > Manager is internal component - a logical brick of the
Service
> > > (e.g.
> > > > > > > RaftGroupManager or TableSchemaManager, TableAffinityManager),
> it
> > > is
> > > > > not
> > > > > > > self-sufficient as affinity or schema make no sense without
the
> > > > table.
> > > > > > > Processor is just helper-component of the Service that
routes
> > > > messages,
> > > > > > > executes async tasks, manages subscriptions and implements
some
> > > > > secondary
> > > > > > > functions.
> > > > > > >
> > > > > > > On Tue, Mar 30, 2021 at 11:24 AM Alexey Goncharuk <
> > > > > > > alexey.goncharuk@gmail.com> wrote:
> > > > > > >
> > > > > > > > Hello Alexander, Igniters,
> > > > > > > >
> > > > > > > > I support the suggestion, we need to work out some
ground
> rules
> > > to
> > > > > > have a
> > > > > > > > consistent naming convention. Agree with having at
most one
> > > > component
> > > > > > per
> > > > > > > > project module - this requirement may turn out to
be too
> strict
> > > in
> > > > > the
> > > > > > > > future, but now it seems reasonable and may help us
to better
> > > > > structure
> > > > > > > the
> > > > > > > > code. Additionally, I would encourage us to make package
> names
> > > > > > consistent
> > > > > > > > with the module's structure to make modules Jigsaw-compliant.
> > We
> > > do
> > > > > not
> > > > > > > > have module definitions now, but I think it would
be great to
> > > have
> > > > > > them,
> > > > > > > it
> > > > > > > > should help us to enforce component boundaries and
proper
> > > > > > responsibility
> > > > > > > > encapsulation.
> > > > > > > >
> > > > > > > > As for the naming, it's not entirely clear for me
when to use
> > the
> > > > > term
> > > > > > > > Service vs Manager. Serice is an entry point to a
> > > component/server,
> > > > > but
> > > > > > > so
> > > > > > > > is Manager - a Manager defines an API that is exposed
by a
> > module
> > > > to
> > > > > > > other
> > > > > > > > modules. Subjectively, I see the following difference
> between a
> > > > > Manager
> > > > > > > and
> > > > > > > > a Service in the examples of entities you provided:
> > > > > > > >  * A Manager is a node singleton. Its whole purpose
is to
> > provide
> > > > an
> > > > > > API
> > > > > > > > gateway for other components into a particular subsystem
of a
> > > node
> > > > > > > >  * A Service is an object that is bound to a particular
> runtime
> > > > > entity
> > > > > > > > (raft group service is bound to a raft group, and
we can have
> > > > > multiple
> > > > > > > Raft
> > > > > > > > groups; partition service is bound to a particular
> partition).
> > We
> > > > can
> > > > > > > > re-create services based on changing runtime state
and/or
> > > > > > configuration.
> > > > > > > > Does this make sense?
> > > > > > > >
> > > > > > > > Finally, I would use lang module name instead of core
(the
> core
> > > is
> > > > > > > > confusing because right now core contains all necessary
> classes
> > > > > > required
> > > > > > > to
> > > > > > > > start a minimal Ignite instance; this sets up wrong
> > expectations
> > > > for
> > > > > > > Ignite
> > > > > > > > 3). Additionally, I think it would be good to exploit
the old
> > > > > > > > org.apache.ignite and org.apache.ignite.internal naming
> scheme:
> > > all
> > > > > > > public
> > > > > > > > classes must go to the non-internal package. The ignite-lang
> > > module
> > > > > > will
> > > > > > > > have both public and internal packages. This automatically
> > > implies
> > > > > that
> > > > > > > all
> > > > > > > > modules except ignite-api and ignite-lang must reside
solely
> in
> > > > > > > > org.apache.ignite.internal.* packages. This will be
easy to
> > check
> > > > and
> > > > > > > > maintain.
> > > > > > > >
> > > > > > > > Throughts?
> > > > > > > >
> > > > > > > > --AG
> > > > > > > >
> > > > > > > > пт, 26 мар. 2021 г. в 20:28, Alexander Lapin
<
> > > lapin1702@gmail.com
> > > > >:
> > > > > > > >
> > > > > > > > > Igniters,
> > > > > > > > >
> > > > > > > > > Seems that within Ignite-3 we have some mess
in terms like
> > > > manager,
> > > > > > > cpu,
> > > > > > > > > service, module, etc. Let's clarify this point.
Also It'll
> be
> > > > great
> > > > > > to
> > > > > > > > > discuss the rules of dividing code into modules.
> > > > > > > > > I'll use the context of Ignite cluster &
node lifecycle
> > > > > > > > > <
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/ignite-3/blob/ignite-14393/modules/runner/README.md
> > > > > > > > > >
> > > > > > > > > for terms definition and as an example source.
> > > > > > > > >
> > > > > > > > > *Terms clarification.*
> > > > > > > > >
> > > > > > > > >    - Component - semantically consistent part
of Ignite
> that
> > in
> > > > > most
> > > > > > > > cases
> > > > > > > > >    will have component-public but ignite-internal
API and a
> > > > > > lifecycle,
> > > > > > > > > somehow
> > > > > > > > >    related to the lifecycle of a node or cluster.
So,
> > > > > *structurally*
> > > > > > > > >    TableManager, SchemaManager, AffinityManager,
etc are
> all
> > > > > > > components.
> > > > > > > > > For
> > > > > > > > >    example, TableManager will have methods like
> > createTable(),
> > > > > > > > > alterTable(),
> > > > > > > > >    dropTable(), etc and a lifecycle that will
create
> > listeners
> > > > (aka
> > > > > > > > >    DistributedMetastorage watches) on schema
and affinity
> > > updates
> > > > > in
> > > > > > > > order
> > > > > > > > > to
> > > > > > > > >    create/drop raft servers for particular partitions
that
> > > should
> > > > > be
> > > > > > > > > hosted on
> > > > > > > > >    local node). Components are lined up in a
graph without
> > > > cycles,
> > > > > > for
> > > > > > > > more
> > > > > > > > >    details please see mentioned above Ignite
cluster & node
> > > > > > lifecycle.
> > > > > > > > >    <
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/ignite-3/blob/ignite-14393/modules/runner/README.md
> > > > > > > > > >
> > > > > > > > >    - Manager is a driving point of a component
with high
> > level
> > > > > > > lifecycle
> > > > > > > > >    logic and API methods. My intention here is
to agree
> about
> > > > > naming:
> > > > > > > > > should
> > > > > > > > >    we use the term Manager, Processor or anything
else?
> > > > > > > > >    - Service is an entry point to some component/server
or
> a
> > > > group
> > > > > of
> > > > > > > > >    components/servers. See RaftGroupService.java
> > > > > > > > >    <
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/ignite-3/blob/main/modules/raft-client/src/main/java/org/apache/ignite/raft/client/service/RaftGroupService.java
> > > > > > > > > >
> > > > > > > > >    as an example.
> > > > > > > > >    - Server, for example RaftServer, seems to
be
> > > self-explanatory
> > > > > > > itself.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > *Dividing code into modules.*
> > > > > > > > > It seems useful to introduce a restriction that
a module
> > should
> > > > > > contain
> > > > > > > > at
> > > > > > > > > most one component. So that, combining component-specific
> > > modules
> > > > > and
> > > > > > > > ones
> > > > > > > > > of api, lang, etc we will end up with something
like
> > following:
> > > > > > > > >
> > > > > > > > >    - affinity // TO be created.
> > > > > > > > >    - api [public]
> > > > > > > > >    - baseline // TO be created.
> > > > > > > > >    - bytecode
> > > > > > > > >    - cli
> > > > > > > > >    - cli-common
> > > > > > > > >    - configuration
> > > > > > > > >    - configuration-annotation-processor
> > > > > > > > >    - core // Module with classes like IgniteUuid.
Should we
> > > > raname
> > > > > it
> > > > > > > to
> > > > > > > > >    lang/utils/commons?
> > > > > > > > >    - metastorage-client // To be created.
> > > > > > > > >    - metastorage-common // To be created.
> > > > > > > > >    - metastorage-server // TO be created.
> > > > > > > > >    - network
> > > > > > > > >    - raft // raft-server?
> > > > > > > > >    - raft-client
> > > > > > > > >    - rest
> > > > > > > > >    - runner
> > > > > > > > >    - schema
> > > > > > > > >    - table // Seems that there might be a conflict
between
> > the
> > > > > > meaning
> > > > > > > of
> > > > > > > > >    table module that we already have and table
module with
> > > > > > > > > create/dropTable()
> > > > > > > > >    - vault
> > > > > > > > >
> > > > > > > > > Also it's not quite clear to me how we should
split lang
> and
> > > util
> > > > > > > classes
> > > > > > > > > some of which belong to the public api, and some
to the
> > > private.
> > > > > > > > >
> > > > > > > > > Please share your thoughts about topics mentioned
above.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > > Alexander
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best regards,
> > > > > > > Andrey V. Mashenkov
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > >
> > > > > > Best regards,
> > > > > > Alexei Scherbakov
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > Best regards,
> > > > Alexei Scherbakov
> > > >
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >
>


-- 

Best regards,
Alexei Scherbakov

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message