Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ public static void main(String[] args) throws Exception {
/**
* Checks that parent process is alive.
*
* <p>This logic is intentionally duplicated here and in {@link IgniteNodeRunner}.
* Compatibility tests may run with an older {@code ignite-core-tests} on the classpath,
* so this runner cannot safely depend on newly added methods in {@code IgniteNodeRunner}.</p>
*
* <p>We listen on {@code System.in} because this compatibility node is started by the parent JVM via
* {@link ProcessBuilder}, where {@code System.in} is a pipe from the parent. When the parent process exits,
* the write-end of the pipe is closed and {@code System.in.read()} returns EOF. We treat this as a signal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,50 @@ public static void main(String[] args) throws Exception {

X.println("Starting Ignite Node... Args=" + Arrays.toString(args));

startParentPipeWatcher();

Comment on lines +70 to +71
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, my idea was to introduce the following structure:

IgniteNodeRunner {
    void run() {
        startParentPipeWatcher();
        doRun();
    }

    void doRun() {
        <custom logic>
    }
}

IgniteCompatibilityNodeRunner {
    @Override void doRun() {
        <custom logic>
    }
}

This approach was implemented in the first commit of this PR.

However, the problem is that IgniteNodeRunner is located in the core module, and for compatibility tests we load its older versions (see IgniteCompatibilityAbstractTest#getDependencies, getExcluded).

As a result, IgniteCompatibilityNodeRunner cannot override methods of older versions of IgniteNodeRunner.

I believe that, for now, it’s better to just copy-paste IgniteCompatibilityNodeRunner#startParentPipeWatcher into IgniteNodeRunner, since it currently has only one inheritor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe describe this problem as comment at IgniteCompatibilityNodeRunner?

IgniteConfiguration cfg = readCfgFromFileAndDeleteFile(args[0]);

ignite = Ignition.start(cfg);
}

/**
* Checks that parent process is alive.
*
* <p>We listen on {@code System.in} because this node runner is started by the parent JVM via
* {@link ProcessBuilder}, where {@code System.in} is a pipe from the parent. When the parent process exits,
* the write-end of the pipe is closed and {@code System.in.read()} returns EOF. We treat this as a signal
* to stop Ignite and terminate the process.</p>
*/
private static void startParentPipeWatcher() {
Thread thread = new Thread(() -> {
try {
while (System.in.read() != -1) {
// No-op
}
}
catch (IOException e) {
X.println("Failed to read parent stdin pipe, stopping Ignite node: " + e);
}

X.println("Parent JVM stdin pipe is closed, stopping Ignite node");

try {
if (ignite != null)
Ignition.stop(ignite.name(), true);
}
catch (Throwable e) {
X.println("Failed to stop Ignite node after parent pipe closure: " + e);
}
finally {
System.exit(0);
}
}, "ignite-parent-pipe-watcher");

thread.setDaemon(true);
thread.start();
}

Comment on lines +77 to +113
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy-pasted from IgniteCompatibilityNodeRunner#startParentPipeWatcher with some comment adjustments

/**
* @return Ignite instance started at main.
*/
Expand Down