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 >
