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
>

Reply via email to