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

[improve]Transform the End-To-End(E2E) tasks on the assembly line #466

Merged
merged 48 commits into from
Sep 4, 2024

Conversation

DongLiang-0
Copy link
Contributor

@DongLiang-0 DongLiang-0 commented Aug 9, 2024

Proposed changes

Issue Number: close #xxx

Problem Summary:

The overall code of our early End-To-End tasks was quite confusing. As a result, when we needed to add a simple case, we often needed to add the entire pipeline code. In this way, the readability and scalability of the code are reduced, so we made this transformation.

Checklist(Required)

  1. Does it affect the original behavior: (Yes/No/I Don't know)
  2. Has unit tests been added: (Yes/No/No Need)
  3. Has document been added or modified: (Yes/No/No Need)
  4. Does it need to update dependencies: (Yes/No)
  5. Are there any changes that cannot be rolled back: (Yes/No)

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@DongLiang-0 DongLiang-0 marked this pull request as draft August 9, 2024 10:42
@DongLiang-0 DongLiang-0 force-pushed the improve-e2e branch 3 times, most recently from ce59eb4 to 137af59 Compare August 15, 2024 07:10
@DongLiang-0 DongLiang-0 changed the title [improve]improve auto ci [Step 1][improve]Transform the End-To-End tasks on the assembly line Aug 15, 2024
@DongLiang-0 DongLiang-0 marked this pull request as ready for review August 15, 2024 07:24
Comment on lines 1 to 8
DROP TABLE IF EXISTS test_e2e_mysql.tbl1;

CREATE TABLE test_e2e_mysql.tbl1 (
`name` varchar(256) primary key,
`age` int
);

insert into test_e2e_mysql.tbl1 values ('doris_1',1);
Copy link
Member

Choose a reason for hiding this comment

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

The table structures are the same, only the table names are different. Do we need to extract them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the next plan. These cases will be transformed. Each table will have a different structure and will be more complex. Now these cases are too simple.

Copy link
Member

Choose a reason for hiding this comment

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

For functional testing, can this table structure be reused? And for all types of tests, a single table can be written. I feel that there are too many identical table structures here, which doesn't seem to make much sense?

@DongLiang-0 DongLiang-0 changed the title [Step 1][improve]Transform the End-To-End tasks on the assembly line [Step 1][improve]Transform the End-To-End(E2E) tasks on the assembly line Aug 15, 2024
@DongLiang-0 DongLiang-0 force-pushed the improve-e2e branch 2 times, most recently from 050b5ff to 3558af3 Compare August 22, 2024 04:23
@DongLiang-0 DongLiang-0 changed the title [Step 1][improve]Transform the End-To-End(E2E) tasks on the assembly line [improve]Transform the End-To-End(E2E) tasks on the assembly line Aug 26, 2024
@DongLiang-0 DongLiang-0 force-pushed the improve-e2e branch 6 times, most recently from 31affcf to 26613bf Compare August 27, 2024 10:18
Comment on lines 76 to 91
public void cancelCurrentJob(String jobName) {
String currentName = currentJobName.get();
Future<?> currentFuture = currentJob.get();

if (currentFuture != null && !currentFuture.isDone() && jobName.equals(currentName)) {
LOG.info("Cancelling the current job. jobName={}", jobName);
boolean cancelled = currentFuture.cancel(true);
if (cancelled) {
LOG.info("Job successfully cancelled. jobName={}", jobName);
} else {
LOG.info("Job cancellation failed. jobName={}", jobName);
}
} else {
LOG.info("No matching job to cancel or job already completed. jobName={}", jobName);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to introduce a thread pool? Why not use Flink's built-in jobClient to cancel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea.

Copy link
Member

@JNSimba JNSimba left a comment

Choose a reason for hiding this comment

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

LGTM

@JNSimba JNSimba merged commit 175f0ff into apache:master Sep 4, 2024
6 checks passed
@JNSimba JNSimba mentioned this pull request Oct 12, 2024
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

Successfully merging this pull request may close these issues.

2 participants