Hi Xinyu, I fully agree with all of this … especially the part where I don’t have to update the Chinese documentation ;-) I guess if I did that, I would have no idea if the end result was really what I intended it to be or a recipe for dumplings ;-)
Happy to whip up a PR based on 1.4.0-SNAPSHOT for config and data-node (I guess it might even make sense to put the shared code in commons … this way I can easily update the AI node to work synchronously. I guess from a user perspective, I guess it would make sense to possibly update the ServerCommandLine to allow a simple “stop” or “remove” command without any additional arguments, that simply removes the node where the code is executed. Should we then also allow removal of multiple nodes by supporting passing in multiple node-ids? I know the existing code indicated that in general removing multiple nodes was planned at some time, but other parts of the code then forced single-node removals. Chris Von: Xinyu Tan <[email protected]> Datum: Mittwoch, 14. August 2024 um 10:35 An: [email protected] <[email protected]> Betreff: Re: AW: [DISCUSS] Refactor ConfigNode and DataNode to be the command-line instances? Hi, Chris Thank you for your sharing, and I salute your ongoing passion for code refactoring. After studying the related code, I believe there are four tasks that need to be done: * Remove xxNodeCommandLine: It seems that these classes are indeed redundant, and their previous multi-round mutual calls with xxNode are confusing. I look forward to your clear refactoring. * Refactor the start-up commands of datanode and confignode using commons cli: The concept of SBOM has been quite popular recently, and IoTDB is currently trying to control the introduction of new dependency packages to avoid some potential compliance risks. I checked our POM file and found that we have already depended on commons cli, so this change will not bring new dependency packages, and I fully support doing this, which will be much more elegant than the current manual judgment of strings. * Change the start and stop commands: Currently, when upgrading, we often recommend that users update the lib directory and the sbin directory synchronously, but I just checked the iotdb official website and did not find any relevant documentation on upgrades. I think as long as we have synchronized updates for all scripts, there will be no problem. I can take charge of promoting the update of user upgrade documentation to avoid problems when users upgrade. * AINode: After AINode is open-sourced in the future, we need to go through the above work again for AINode. What are your thoughts? Best ------------- Xinyu Tan On 2024/08/12 08:06:34 Christofer Dutz wrote: > I think it’s unfortunately a bit core to many of the design-decisions being > made in IoTDB. > Many tasks seem to be done in too short time and quite often people are not > taking the required “step back” in order to create a good overall > architecture. > I am seeing this in way too many places in the config-node and data-node > modules. > > My proposal would be to completely rewrite the code for the starting and > stopping of the data- and config nodes. This wouldn’t be as much work as it > sounds, a I would personally base it on Apache Commons CLI. This way we would > have a much cleaner and better tested command-line. > > However, the syntax would change. > > “DataNode -s” > > Would become something like: > > “DataNode start” > > And: > > “DataNode -r 42” > > Would become something like: > > “DateNode -nodeId 42 remove” > > > Chris > > Von: liyuheng <[email protected]> > Datum: Freitag, 9. August 2024 um 05:54 > An: [email protected] <[email protected]> > Betreff: Re: [DISCUSS] Refactor ConfigNode and DataNode to be the > command-line instances? > Hi Chris, > > I also feel that extracting separate DataNodeCommandLine and > ConfigNodeCommandLine classes is not really necessary. > > When observing the DataNode startup process, although the main function is in > DataNode, it needs to first go around to DataNodeCommandLine and then back to > DataNode. This convoluted call process confuses me. Perhaps this is an > attempt to separate the command line from the main class? Or maybe this is > just a history design? > > Anyway, I'm looking forward to you simplifying the logic here. > > LiYuHeng > > > 2024年8月7日 22:42,Christofer Dutz <[email protected]> 写道: > > > > Hi all, > > > > while working on cleaning up on some config-node removal, I am struggling a > > bit with the structure of our code. > > For example, the main entry point for a program is “DataNode” and > > “ConfigNode”, but the main methods, redirect control to a “commandline” > > instance. > > Depending on the command issued, the commandline then needs to reference > > back to the same instance that called the commandLine do to some other > > things. > > > > I think as both DataNode and ConfigNode only implement an mbean interface, > > it would be cleaner to move the CLI stuff into the main classes and move > > the stuff for starting and stopping nodes into “command” implementations > > for the CLI? This mixture is hard to test and maintain. > > > > Also, could the think about basing the CLI on Apache Commons CLI, however > > that could bring changes in output with it. > > > > What do you others think? > > > > Chris >
