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.goncha...@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.scherbak...@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.goncha...@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.scherbak...@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.goncha...@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.scherbak...@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.mashen...@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.goncha...@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 < > > > lapin1...@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
