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

[Fix] [Bug] [Upgrade] Resolved no data on the task instance page after upgrading from 1.3.x to 3.1.x #15498

Merged
merged 6 commits into from
Jan 26, 2024

Conversation

pinkfloyds
Copy link
Contributor

no data on the task instance page after upgrading from 1.3.4 to 3.1.x

modify the update script to change t_ds_task_instance.task_definition_version default to 1 .
see #15497

@mergeable mergeable bot removed the backend label Jan 16, 2024
@pinkfloyds pinkfloyds changed the title [Fix] [BUG] [Upgrade] Resolved no data on the task instance page after upgrading from 1.3.x to 3.1.x [Fix] [Bug] [Upgrade] Resolved no data on the task instance page after upgrading from 1.3.x to 3.1.x Jan 16, 2024
@@ -415,7 +415,7 @@ alter table t_ds_process_instance drop dependence_schedule_times;

-- t_ds_task_instance note: Data migration is not supported
alter table t_ds_task_instance change process_definition_id task_code bigint(20) NOT NULL COMMENT 'task definition code';
alter table t_ds_task_instance add task_definition_version int(11) DEFAULT '0' COMMENT 'task definition version' AFTER task_code;
alter table t_ds_task_instance add task_definition_version int(11) DEFAULT '1' COMMENT 'task definition version' AFTER task_code;
Copy link
Member

Choose a reason for hiding this comment

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

The default value of t_ds_task_instance is '0', we cannot change this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or I could add it here ?
update t_ds_task_instance set task_definition_version ='1';

Copy link
Member

@ruanwenjun ruanwenjun Jan 20, 2024

Choose a reason for hiding this comment

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

LGTM only update the task_definition_version, but the version in t_ds_task_definition, t_ds_task_definition_log should start from 0, after you upgrade there should only contains one version in these two table, this is strange.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

t_ds_task_inset.task_definition_version is assigned to 1 for the first time, which is not useful when the default value is 0,the data in the table should not have t_ds_task_instance task_definition_version = 0,so update t_ds_task_instance set task_definition_version ='1'

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see the code will set the begin version to 0. So set the default version to 1 is the correct way.
Could you please change the default value in the initialize sql.
https://github.com/apache/dolphinscheduler/tree/dev/dolphinscheduler-dao/src/main/resources/sql
And we also need to alter this latest version https://github.com/apache/dolphinscheduler/tree/dev/dolphinscheduler-dao/src/main/resources/sql/upgrade/3.2.1_schema.

Comment on lines 17 to 18
alter table t_ds_process_definition alter column `version` set default 1;
alter table t_ds_process_definition alter column `version` int(11) NOT NULL;
Copy link
Member

Choose a reason for hiding this comment

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

alter table t_ds_process_definition modify column `version` int(11) NOT NULL default 1;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,have been modified

Copy link
Contributor Author

@pinkfloyds pinkfloyds left a comment

Choose a reason for hiding this comment

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

done

@ruanwenjun
Copy link
Member

image

@SbloodyS SbloodyS added this to the 3.2.1 milestone Jan 24, 2024
@pinkfloyds
Copy link
Contributor Author

Prompt to add the ready-to-merge label. Can you add it? I can't find an action to add a label

@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (419cb49) 38.10% compared to head (5d94021) 38.10%.

❗ Current head 5d94021 differs from pull request most recent head 91bd976. Consider uploading reports for the commit 91bd976 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##                dev   #15498   +/-   ##
=========================================
  Coverage     38.10%   38.10%           
  Complexity     4698     4698           
=========================================
  Files          1304     1304           
  Lines         44818    44818           
  Branches       4804     4804           
=========================================
  Hits          17080    17080           
  Misses        25884    25884           
  Partials       1854     1854           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

sonarcloud bot commented Jan 24, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

ruanwenjun
ruanwenjun previously approved these changes Jan 24, 2024
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

sonarcloud bot commented Jan 26, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@ruanwenjun ruanwenjun merged commit e20e066 into apache:dev Jan 26, 2024
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants