dbeaver/pro#2623 Timeout settings for tasks #32373

serjiokov · 2024-05-14T07:01:17Z

The new property "Max execution time" is specified for task automatic cancelation.
Handled and processed statuses.

git@github.com:dbeaver/dbeaver.git into dbeaver/pro#2795-editor-restore-path

Added service job implementation, ui settings, cancellation logic.

Handle database task status mechanics

Code cleanup

git@github.com:dbeaver/dbeaver.git into dbeaver/pro#2623-task-timeout-job

serjiokov · 2024-05-14T07:01:52Z

serjiokov · 2024-05-14T07:02:10Z

serjiokov · 2024-05-14T07:03:11Z

There are several cancelation cases supported:
1- Cancelation by user
2- Cancelation by timeout reached.

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

E1izabeth · 2024-05-14T07:16:26Z

What if user put -1 into this field? Please check

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

serge-rider · 2024-05-14T10:24:31Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

@@ -325,9 +328,33 @@ public DBTTaskRunStatus runTask(@NotNull DBRProgressMonitor monitor, @NotNull DB
    public TaskRunJob scheduleTask(@NotNull DBTTask task, @NotNull DBTTaskExecutionListener listener) {
        final TaskRunJob runJob = createJob((TaskImpl) task, listener);
        runJob.schedule();
+        if (serviceJob == null) {
+            serviceJob = createServiceJob();
+            serviceJob.setSystem(true);


Move it to job constructor

serge-rider · 2024-05-14T10:25:03Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

+            @Override
+            protected IStatus run(
+                IProgressMonitor monitor) {
+                for (TaskRunJob taskJob : runningTasks) {


Race condition may happen. Create copy of running tasks first and then iterate.

serge-rider · 2024-05-14T10:25:12Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

+                        taskJob.setCancelByExecutionTime();
+                    }
+                }
+                schedule(1000);


Make 1000 a constant

ShadelessFox · 2024-05-14T10:21:43Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

+            protected IStatus run(
+                IProgressMonitor monitor) {


Or rather

Suggested change

protected IStatus run(

IProgressMonitor monitor) {

protected IStatus run(IProgressMonitor monitor) {

ShadelessFox · 2024-05-14T10:23:46Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

+                    if (taskJob.checkCancelation()) {
+                        taskJob.setCancelByExecutionTime();
+                    }


Why are you delegating checks and cancellations to the callee? Do everything within one call in the job itself.

ShadelessFox · 2024-05-14T10:26:01Z

...org.jkiss.dbeaver.tasks.ui/src/org/jkiss/dbeaver/tasks/ui/internal/TaskUIMessages.properties

@@ -36,6 +36,9 @@ task_config_wizard_page_task_control_label_descr = Description
 task_config_wizard_page_task_text_label_task_id = Task ID
 task_config_wizard_page_task_create_folder_label = Task folder
 task_config_wizard_page_task_no_task_types_available = No suitable task types found for the current project.\nPlease create a new connection to continue.
+task_config_wizard_page_task_advanced_label = Advanced
+task_config_wizard_page_task_max_exec_time = Max execution time (in seconds):
+task_config_wizard_page_task_max_exec_time_descr = The time require to execute certain task, after it expired the task will be terminated.


Suggested change

task_config_wizard_page_task_max_exec_time_descr = The time require to execute certain task, after it expired the task will be terminated.

task_config_wizard_page_task_max_exec_time_descr = The timeout before the task will be terminated.

ShadelessFox · 2024-05-14T10:26:55Z

....dbeaver.tasks.ui/src/org/jkiss/dbeaver/tasks/ui/wizard/TaskConfigurationWizardPageTask.java

+                public void widgetSelected(
+                    SelectionEvent e) {


Suggested change

public void widgetSelected(

SelectionEvent e) {

public void widgetSelected(SelectionEvent e) {

Or rather use org.eclipse.swt.events.SelectionListener#widgetSelectedAdapter

ShadelessFox · 2024-05-14T10:27:50Z

....dbeaver.tasks.ui/src/org/jkiss/dbeaver/tasks/ui/wizard/TaskConfigurationWizardPageTask.java

+            if (maxExecutionTimeBtn.getSelection()) {
+                task.setMaxExecutionTime(CommonUtils.toInt(maxExecutionTime.getText()));
+            } else {
+                task.setMaxExecutionTime(CommonUtils.toInt(0));


Suggested change

task.setMaxExecutionTime(CommonUtils.toInt(0));

task.setMaxExecutionTime(0);

ShadelessFox · 2024-05-14T10:29:38Z

....dbeaver.tasks.ui/src/org/jkiss/dbeaver/tasks/ui/wizard/TaskConfigurationWizardPageTask.java

@@ -430,6 +463,11 @@ public void saveSettings() {
                }
                TaskRegistry.getInstance().notifyTaskFoldersListeners(new DBTTaskFolderEvent(folder, DBTTaskFolderEvent.Action.TASK_FOLDER_REMOVE));
            }
+            if (maxExecutionTimeBtn.getSelection()) {
+                task.setMaxExecutionTime(CommonUtils.toInt(maxExecutionTime.getText()));


Suggested change

task.setMaxExecutionTime(CommonUtils.toInt(maxExecutionTime.getText()));

task.setMaxExecutionTime(maxExecutionTime.getSelection());

ShadelessFox · 2024-05-14T10:31:39Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

        return runJob;
    }

+    private Job createServiceJob() {
+        return new Job("Service canceling job") {


Suggested change

return new Job("Service canceling job") {

return new Job("Task canceling job") {

ShadelessFox · 2024-05-14T10:32:28Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

@@ -394,6 +421,7 @@ private void loadConfiguration() {
                    String taskFolderName = JSONUtils.getString(taskJSON, TaskConstants.TAG_TASK_FOLDER);
                    Date createTime = systemDateFormat.parse(JSONUtils.getString(taskJSON, TaskConstants.TAG_CREATE_TIME));
                    Date updateTime = systemDateFormat.parse(JSONUtils.getString(taskJSON, TaskConstants.TAG_UPDATE_TIME));
+                    Integer maxExecutionTime = JSONUtils.getInteger(taskJSON, TaskConstants.TAG_MAX_EXEC_TIME);


Suggested change

Integer maxExecutionTime = JSONUtils.getInteger(taskJSON, TaskConstants.TAG_MAX_EXEC_TIME);

int maxExecutionTime = JSONUtils.getInteger(taskJSON, TaskConstants.TAG_MAX_EXEC_TIME);

ShadelessFox · 2024-05-14T10:33:22Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

@@ -545,6 +574,7 @@ private void serializeTasks(@NotNull JsonWriter jsonWriter) throws IOException {
            JSONUtils.field(jsonWriter, TaskConstants.TAG_CREATE_TIME, systemDateFormat.format(task.getCreateTime()));
            JSONUtils.field(jsonWriter, TaskConstants.TAG_UPDATE_TIME, systemDateFormat.format(task.getUpdateTime()));
            JSONUtils.serializeProperties(jsonWriter, TaskConstants.TAG_STATE, task.getProperties());
+            JSONUtils.field(jsonWriter, TaskConstants.TAG_MAX_EXEC_TIME, task.getMaxExecutionTime());


I'd not write this property if it's x <= 0 to avoid cluttering configuration files

ShadelessFox · 2024-05-14T10:35:23Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskRunJob.java

+        if (task.getMaxExecutionTime() > 0) {
+            if ((System.currentTimeMillis() - taskStartTime) > (task.getMaxExecutionTime() * 1000)) {
+                return true;
+            }
+        }
+        return false;


Suggested change

if (task.getMaxExecutionTime() > 0) {

if ((System.currentTimeMillis() - taskStartTime) > (task.getMaxExecutionTime() * 1000)) {

return true;

}

}

return false;

return task.getMaxExecutionTime() > 0 && System.currentTimeMillis() - taskStartTime > task.getMaxExecutionTime() * 1000;

serjiokov · 2024-05-14T12:02:52Z

MashaKorax · 2024-05-14T14:19:05Z

Not working for tasks with scheduling

serge-rider · 2024-05-16T13:25:28Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

+            serviceJob.schedule();
+        }
+        try {
+            job.join();


You changed core behavior.
We mustn't schedule new job here.
We mustn't call job.join() anywhere.

job.join()

ShadelessFox · 2024-05-17T09:45:24Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskRunJob.java

+                if (activeMonitor instanceof TaskLoggingProgressMonitor tlpm) {
+                    tlpm.setCanceled(true);
+                }


Why not cancel any monitor?

ShadelessFox · 2024-05-17T09:46:32Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

        if (error != null) {
            if (error instanceof DBException e) {
                throw e;
            } else {
                throw new DBException("Error executing task", error);
            }
        }
+        runningTasks.remove(job);


This won't be executed if job fails with an error.

ShadelessFox · 2024-05-17T09:48:50Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java

@@ -325,9 +335,13 @@ public DBTTaskRunStatus runTask(@NotNull DBRProgressMonitor monitor, @NotNull DB
    public TaskRunJob scheduleTask(@NotNull DBTTask task, @NotNull DBTTaskExecutionListener listener) {
        final TaskRunJob runJob = createJob((TaskImpl) task, listener);
        runJob.schedule();
+        if (serviceJob == null) {


The runningTasks.add(job); is missing here. Is this expected?

The management of runningTasks goes from job listener, method abouteToRun

…eaver/dbeaver.git into dbeaver/pro#2623-task-timeout-job

MashaKorax · 2024-05-21T14:05:33Z

verified

ShadelessFox · 2024-05-21T14:26:54Z

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskRunJob.java

+            }
+            if (taskStatus.getResultMessage() == null) {


Suggested change

}

if (taskStatus.getResultMessage() == null) {

} else if (taskStatus.getResultMessage() == null) {

serjiokov added 8 commits May 13, 2024 15:43

Merge branch 'dbeaver/pro#2795-editor-restore-path' of

d19ea88

git@github.com:dbeaver/dbeaver.git into dbeaver/pro#2795-editor-restore-path

dbeaver/pro#2623 - Timeout settings for tasks

dd61823

Added service job implementation, ui settings, cancellation logic.

dbeaver/pro#2623 - Timeout settings for tasks

8c44c25

Handle database task status mechanics

dbeaver/pro#2623 - Timeout settings for tasks

c317278

Code cleanup

Merge branch 'dbeaver/pro#2623-task-timeout-job' of

45d8722

git@github.com:dbeaver/dbeaver.git into dbeaver/pro#2623-task-timeout-job

'dbeaver/pro#2623 Code cleanup

37a1af2

'dbeaver/pro#2623 Code cleanup

6564668

'dbeaver/pro#2623 Code cleanup

606d6a7

E1izabeth reviewed May 14, 2024

View reviewed changes

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java Outdated Show resolved Hide resolved

serjiokov added 4 commits May 14, 2024 11:16

dbeaver/pro#2623 Reworked on centralized monitoring service job

599ac58

dbeaver/pro#2623 Seconds to Milliseconds

4ac0dd3

dbeaver/pro#2623 Cancel by timeout fixed

f3e745f

dbeaver/pro#2623 Cancel by timeout fix

58af8c0

E1izabeth reviewed May 14, 2024

View reviewed changes

plugins/org.jkiss.dbeaver.registry/src/org/jkiss/dbeaver/registry/task/TaskManagerImpl.java Outdated Show resolved Hide resolved

E1izabeth requested a review from ShadelessFox May 14, 2024 10:05

serjiokov added 3 commits May 14, 2024 12:07

dbeaver/pro#2623 Java doc fix

384f3b5

dbeaver/pro#2623 System job fix

6ed4fff

dbeaver/pro#2623 Rework to use Spinner

2c7e1f9

serge-rider requested changes May 14, 2024

View reviewed changes

ShadelessFox reviewed May 14, 2024

View reviewed changes

serjiokov and others added 6 commits May 14, 2024 12:59

dbeaver/pro#2623 Thread safe check added

b510c8a

dbeaver/pro#2623 Minor changes

c74b61c

dbeaver/pro#2623 Review

2d5303f

Merge branch 'devel' into dbeaver/pro#2623-task-timeout-job

2167ca3

dbeaver/pro#2623 - Aggregated cancel invoke

e006a0b

dbeaver/pro#2623 - Minor style fix

91eff05

serjiokov and others added 9 commits May 14, 2024 18:04

dbeaver/pro#2623 - Schedule tasks fix

c6e1a4f

dbeaver/pro#2623 - Schedule tasks fix

1f25b81

dbeaver/pro#2623 - Schedule tasks fix

60c51e0

Merge branch 'devel' into dbeaver/pro#2623-task-timeout-job

462cab9

dbeaver/pro#2623 - Logging data added

d1aef70

Merge branch 'devel' into dbeaver/pro#2623-task-timeout-job

cf00a03

dbeaver/pro#2623-task-timeout-job Fixed logging start task

b010670

dbeaver/pro#2623-task-timeout-job Grammar issue

8b78b3d

Merge branch 'devel' into dbeaver/pro#2623-task-timeout-job

4517e57

serge-rider requested changes May 16, 2024

View reviewed changes

serjiokov added 3 commits May 17, 2024 10:40

dbeaver/pro#2623-task-timeout-job Implementation for CLI mode avoid

8e0ec00

job.join()

dbeaver/pro#2623 - Unrelated changes

5a569ca

dbeaver/pro#2623 - Fixed cancellation in Loggining progress monitor

5d4585d

ShadelessFox reviewed May 17, 2024

View reviewed changes

serjiokov and others added 5 commits May 17, 2024 12:03

dbeaver/pro#2623 - Minor fix

9f326be

Merge branch 'devel' into dbeaver/pro#2623-task-timeout-job

1de5e65

dbeaver/pro#2623 - Status message for composite task fix

4429f98

Merge branch 'dbeaver/pro#2623-task-timeout-job' of git@github.com:db…

307979d

…eaver/dbeaver.git into dbeaver/pro#2623-task-timeout-job

Merge branch 'devel' into dbeaver/pro#2623-task-timeout-job

5d906da

ShadelessFox reviewed May 21, 2024

View reviewed changes

serge-rider merged commit d0a0b80 into devel May 21, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dbeaver/pro#2623 Timeout settings for tasks #32373

dbeaver/pro#2623 Timeout settings for tasks #32373

serjiokov commented May 14, 2024

serjiokov commented May 14, 2024

serjiokov commented May 14, 2024

serjiokov commented May 14, 2024

E1izabeth commented May 14, 2024 •

edited

serge-rider May 14, 2024

serge-rider May 14, 2024

serge-rider May 14, 2024

ShadelessFox May 14, 2024

ShadelessFox May 14, 2024

ShadelessFox May 14, 2024

ShadelessFox May 14, 2024

ShadelessFox May 14, 2024

serjiokov May 14, 2024

ShadelessFox May 14, 2024

serjiokov May 14, 2024

ShadelessFox May 14, 2024

serjiokov May 14, 2024

ShadelessFox May 14, 2024

ShadelessFox May 14, 2024

serjiokov May 14, 2024

ShadelessFox May 14, 2024

serjiokov May 14, 2024 •

edited

serjiokov commented May 14, 2024

MashaKorax commented May 14, 2024

serge-rider May 16, 2024

ShadelessFox May 17, 2024

ShadelessFox May 17, 2024

ShadelessFox May 17, 2024

serjiokov May 17, 2024

MashaKorax commented May 21, 2024

ShadelessFox May 21, 2024

	protected IStatus run(
	IProgressMonitor monitor) {
	protected IStatus run(IProgressMonitor monitor) {

	task_config_wizard_page_task_max_exec_time_descr = The time require to execute certain task, after it expired the task will be terminated.
	task_config_wizard_page_task_max_exec_time_descr = The timeout before the task will be terminated.

	public void widgetSelected(
	SelectionEvent e) {
	public void widgetSelected(SelectionEvent e) {

	task.setMaxExecutionTime(CommonUtils.toInt(0));
	task.setMaxExecutionTime(0);

	task.setMaxExecutionTime(CommonUtils.toInt(maxExecutionTime.getText()));
	task.setMaxExecutionTime(maxExecutionTime.getSelection());

	return new Job("Service canceling job") {
	return new Job("Task canceling job") {

	Integer maxExecutionTime = JSONUtils.getInteger(taskJSON, TaskConstants.TAG_MAX_EXEC_TIME);
	int maxExecutionTime = JSONUtils.getInteger(taskJSON, TaskConstants.TAG_MAX_EXEC_TIME);

	}
	if (taskStatus.getResultMessage() == null) {
	} else if (taskStatus.getResultMessage() == null) {

dbeaver/pro#2623 Timeout settings for tasks #32373

dbeaver/pro#2623 Timeout settings for tasks #32373

Conversation

serjiokov commented May 14, 2024

serjiokov commented May 14, 2024

serjiokov commented May 14, 2024

serjiokov commented May 14, 2024

E1izabeth commented May 14, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serjiokov May 14, 2024 • edited

Choose a reason for hiding this comment

serjiokov commented May 14, 2024

MashaKorax commented May 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MashaKorax commented May 21, 2024

Choose a reason for hiding this comment

E1izabeth commented May 14, 2024 •

edited

serjiokov May 14, 2024 •

edited