Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent naming of tasks in GridScheduler #507

Open
andrii0lomakin opened this issue Jul 25, 2024 · 2 comments
Open

Inconsistent naming of tasks in GridScheduler #507

andrii0lomakin opened this issue Jul 25, 2024 · 2 comments

Comments

@andrii0lomakin
Copy link
Contributor

Checking of the presence of WorkerGrid in GridScheduler is implemented inconsistently:

  1. In uk.ac.manchester.tornado.runtime.tasks.TornadoTaskGraph#isTaskNamePresent check name of the task is concatinated with the name of the task graph, and then it is compared with a key stored in GridScheduler.
private boolean isTaskNamePresent(String taskName) {
        for (TaskPackage taskPackage : taskPackages) {
            if (taskName.equals(STR."\{taskGraphName}.\{taskPackage.getId()}")) {
                return true;
            }
        }
        return false;
    }

So, it is forcing the user to prepend the name of the task graph with the name of the task during the addition of WorkerGrid to the GridScheduler.
2. In method uk.ac.manchester.tornado.api.GridScheduler#contains the name of the task graph is added by GridScheduler itself, so if the user follows that first approach, this method always returns false.

From my point of view, the first case is implemented incorrectly because, at the current stage, there is no validation of the correctness of the fact that the user provided the correct name to the GridScheduler, and there is no clear demand to do that in the GridScheduler contract.

So IMHO, both cases should be implemented over the call to GridScheduler#contains, and usage of the same scheduler for two task graphs at once should be prohibited (which is quite simple to implement).

@jjfumero
Copy link
Member

If the isTaskNamePresent method returns false, you will get a Runtime Exception. I do not get why this is inconsistent. Could you please share a test case that breaks?

@andrii0lomakin
Copy link
Contributor Author

andrii0lomakin commented Jul 25, 2024

@jjfumero yes you are right (I admit I missed this because encountered this issue long ago), but exception says Grid scheduler with name \{gridName} not found in the Task-Graph , while user clearly sees that she adds this task in task graph with the same name as in GridScheduler. IMHO that is confusing, I for example had to debug to understand root cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants