Hi Maxim, I like the idea, the IEP looks very useful.
However, I agree with Nikolay on this > > Don’t think we should rely on auto scan class path capabilities. > 1. How this automatic scanner will distinguish ignite class from > user class if they both in node class path and same package like > «org.apache.ignite.internal»? > 2. It seems hard to debug any errors with auto class path scanner. > How we ensure correct order of scanning in case different jar order in > class path? I think we should go with some kind of hardcoded «on startup» command > registration. Maybe we could use Java ServiceLoader [1] for this? You can add a list of command classes to e.g. META-INF/services/o.a.i.CommandHandlerProvider, then register all mentioned classes that are found via ServiceLoader.load( CommandHandlerProvider.class) Service loader can return the iterator for all found providers. This will prevent scanning all the classes in classpath and allow to load classes from 3-rd party extensions. [1] https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html On Fri, Nov 26, 2021 at 12:25 PM Nikolay Izhikov <nizhi...@apache.org> wrote: > Hello, Maxim. > > Thanks for the IEP. > I support the intentions and design of the IEP in general but have some > comments: > > > AnnotationCommandScanner, PackageCommandScanner, URICommandScanner > > Don’t think we should rely on auto scan class path capabilities. > 1. How this automatic scanner will distinguish ignite class from > user class if they both in node class path and same package like > «org.apache.ignite.internal»? > 2. It seems hard to debug any errors with auto class path scanner. > How we ensure correct order of scanning in case different jar order in > class path? > > I think we should go with some kind of hardcoded «on startup» command > registration. > > > > execute(ProxyManagementTask.class, Map<String, String> atts) > > 1. Do we have some well-defined place to validate `atts`? > > 2. Do we really need to rely on `java.lang.Class` parameter? > This means executor (thin client) should have all class > definitions which can be hard for plugins provided class. > Can we rely on some kind of «path array» like `String[] { > «baseline», «set» }` or `String[] { «user», «add» }` > So the API usage will looks like: > > ``` > String execute(String[] path, Map<String, String> params) > > Map<String, String> params = new HashMap(); > > params.put(«—login», «admin») > params.put(«—password», «mypassw0rd») > > String res = execute(new String[] {«user», «add»}, params); > ``` > > > > Design issues … it is not possible to add and run new management > tasks at the cluster runtime (e.g. REPL mode for CLI tools can't be used); > > It seems to me as a good thing? > We shouldn’t have ability to add management tasks in runtime from the user > side because of security reasons. > > But, we should be able to run any existing task dynamically in some > scripting environment - REPL, CLI as you mentioned. > > > > Features > > I think we should focus on: > > 1. Subsystem then stores all available management tasks > 2. Unified execution flow that guarantees all required things like > authorization, authentication, logging and event generation. > 3. Creation of the specific protocol implementation that will expose all > existing commands **in the same way** through all supported protocols > **without any additional development**. > 4. Simplification of new command development and improvement of existing. > > > > Compatibility > > IEP doesn’t clearly define compatibility. > Do we keep all existing commands as is? > Is new subsystem a replacement for current API? > > Is new subsystem will co-exist with current API? For how long? > > > 26 нояб. 2021 г., в 11:44, Maxim Muzafarov <mmu...@apache.org> > написал(а): > > > > Igniters, > > > > > > I'd like to propose a new internal cluster management API that will > > simplify new cluster management commands creation and expose new > > commands to CLI, REST, JMX Ignite interfaces without any additional > > actions from the engineer adding a new command. This main goal of the > > IEP is supposed to be available after the implementation of the 1-st > > phase. From my point of view, the implementation will also provide > > some additional features like auto ASCII-docs generation which can be > > achieved with minor code changes. > > > > Please, take a look at the IEP-81 [1] with a detailed explanation of > > my proposal. Below you can find some crucial points from it. > > > > > > = Current Limitations = > > > > - The same commands through CLI, REST, JMX APIs don't have the same > > input arguments and use different subsystems for command execution; > > - A new command that is added must be manually exposed to all the > > Ignite APIs (new implementation required for each new command being > > added); > > - New commands can't be added via Ignite Plugins and exposed to API; > > - The own binary protocol is used (GridClient) for command executions > > instead of the ignite thin client (IgniteClient); > > - Security and role model: a user have to add compute tasks > > permissions the same time as adding permissions for the process he > > intended to use. > > - New commands can't be added or executed at runtime > > > > > > = Crutial Impelemntation Notes = > > > > 1. Create a one for all proxy compute task gate that will accept a map > > of parameters for preparing management command based on input > > parameters and executing it on ignite nodes. > > For instance: > > > > IgniteClient.compute().execute(ProxyManagementTask.class.getName(), > attrs); > > > > Map: > > baseline.add=execute > > baseline.add.projection=SINGLE > > baseline.add.nodes=[consistendtId3, consistendeId4] > > > > 2. Create a CommandRegisty that will contain all available commanded > > on the local ignite node. Commands will be added by command scanners > > e.g. AnnotationCommandScanner, PackageCommandScanner, > > URICommandScanner as well as registered manually at runtime by calling > > `add` method on command registry. The CommandRegistry will also be > > available for the thin clients in a standalone mode. > > > > 3. Prepare a command parsers for REST, JMX, CLI interfaces that will > > use command registry and calling the proxy management task right away > > with correct task input parameters. > > > > 4. Check the API [2] for commands that may be executed only on a > > single node (the same as the VisorOneNodeTask) or on all nodes e.g. > > collecting some information from each node and reducing it on a > > originating node. > > > > > > [1] > https://cwiki.apache.org/confluence/display/IGNITE/IEP-81+Cluster+Management+API > > [2] > https://cwiki.apache.org/confluence/display/IGNITE/IEP-81+Cluster+Management+API#IEP81ClusterManagementAPI-CommandInterface > > -- Best regards, Andrey V. Mashenkov
