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.

Sign up for GitHub

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

Jump to bottom

[Fix-9221] [alert-server] optimization and gracefully close #9246

Merged
merged 17 commits into from
Apr 8, 2022
Merged

[Fix-9221] [alert-server] optimization and gracefully close #9246

merged 17 commits into from
Apr 8, 2022

Conversation

guoshupei
Copy link
Contributor

This closes #9221

Purpose of the pull request

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2022 •
edited
Loading

Codecov Report

Merging #9246 (0a9427c) into dev (4a29c6a) will decrease coverage by 0.04%.
The diff coverage is 46.42%.

@@             Coverage Diff              @@
##                dev    #9246      +/-   ##
============================================
- Coverage     39.98%   39.94%   -0.05%     
- Complexity     4375     4386      +11     
============================================
  Files           822      823       +1     
  Lines         32924    32987      +63     
  Branches       3656     3661       +5     
============================================
+ Hits          13165    13176      +11     
- Misses        18532    18586      +54     
+ Partials       1227     1225       -2
Impacted Files Coverage Δ
...er/api/service/impl/TaskDefinitionServiceImpl.java 24.01% <ø> (+0.05%) ⬆️
.../org/apache/dolphinscheduler/common/Constants.java 80.95% <ø> (ø)
...che/dolphinscheduler/common/enums/AlertStatus.java 0.00% <0.00%> (ø)
.../remote/command/alert/AlertSendRequestCommand.java 88.46% <0.00%> (-3.54%) ⬇️
...che/dolphinscheduler/alert/AlertSenderService.java 54.40% <14.70%> (ø)
...org/apache/dolphinscheduler/alert/AlertServer.java 51.35% <40.90%> (-8.11%) ⬇️
...e/dolphinscheduler/dao/entity/AlertSendStatus.java 55.88% <55.88%> (ø)
...che/dolphinscheduler/alert/AlertPluginManager.java 93.75% <100.00%> (ø)
.../dolphinscheduler/alert/AlertRequestProcessor.java 93.33% <100.00%> (ø)
...java/org/apache/dolphinscheduler/dao/AlertDao.java 27.27% <100.00%> (+5.53%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bab12f...0a9427c. Read the comment docs.

@guoshupei
Copy link
Contributor Author

guoshupei commented Mar 30, 2022 •
edited
Loading

@caishunfeng Hi，can you help me? What is the reason for the failure of the E2E/Tenant check？And I don't have a valid Docker environment, so I am also failure of the E2E/Tenant check in local

@caishunfeng
Copy link
Contributor

caishunfeng commented Mar 30, 2022 •
edited
Loading

@caishunfeng Hi，can you help me? What is the reason for the failure of the E2E/Tenant check？And I don't have a valid Docker environment, so I am also failure of the E2E/Tenant check in local

image
you can check the e2e detail log.

@guoshupei
Copy link
Contributor Author

@caishunfeng Hi，can you help me? What is the reason for the failure of the E2E/Tenant check？And I don't have a valid Docker environment, so I am also failure of the E2E/Tenant check in local

image you can check the e2e detail log.

yes, I saw this error message, I have no problem starting locally, so checkPluginDefineTableExist is passed, but I don't understand how it is verified on git, and I don't know how to solve it, I have no idea
image

@caishunfeng
Copy link
Contributor

I had rerun the CI test.

@guoshupei
Copy link
Contributor Author

guoshupei commented Mar 30, 2022 via email

@guoshupei
Copy link
Contributor Author

Hi, @caishunfeng , finally succeeded, I replace @Eventlistener with @PostConstruct in AlertServer

@caishunfeng
Copy link
Contributor

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 4 Bugs Vulnerability A 0 Vulnerabilities Security Hotspot A 0 Security Hotspots Code Smell A 3 Code Smells

56.6% 56.6% Coverage 5.9% 5.9% Duplication

Please take attention to snoar check.

@guoshupei
Copy link
Contributor Author

guoshupei commented Mar 31, 2022 via email

@zhuangchong
Copy link
Contributor

@guoshupei Please see the review above.

@guoshupei
Copy link
Contributor Author

@guoshupei Please see the review above.

Hi, @zhuangchong, sorry, I am using it for the first time, not very familiar with the operation. I have already replied, it seems that you did not receive it. should I click the button of Resolve conversation?
image

@zhuangchong
Copy link
Contributor

@guoshupei If it has been resolved, click the Resolve conversation button, but I did not see your reply on it. I see that there is a reply in the screenshot you sent, please check it.

image

@guoshupei
Copy link
Contributor Author

@guoshupei If it has been resolved, click the Resolve conversation button, but I did not see your reply on it. I see that there is a reply in the screenshot you sent, please check it.

image

@zhuangchong, I know the reason, I need click submit review button in View changes page

@sonarcloud
Copy link

sonarcloud bot commented Apr 6, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

51.9% 51.9% Coverage
0.8% 0.8% Duplication

zhuangchong
zhuangchong approved these changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@zhuangchong zhuangchong left a comment

Choose a reason for hiding this comment

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

+1

@caishunfeng caishunfeng merged commit ca95d2f into apache:dev Apr 8, 2022
caishunfeng pushed a commit to caishunfeng/dolphinscheduler that referenced this pull request Apr 11, 2022
@guoshupei @caishunfeng
[Fix-9221] [alert-server] optimization and gracefully close (apache#9246
e12ddd6 
)

* [Fix-9221] [alert-server] optimization and gracefully close

This closes apache#9221

* [Fix-9221] [alert-server] remove unused mock data

This closes apache#9221

* [Fix-9221] [alert-server] remove unused mock data

This closes apache#9221

* [Fix-9221] [alert-server] remove unnecessary Mockito stubbings

* [Fix-9221] [alert-server] init AlertPluginManager in AlertServer

* [Fix-9221] [alert-server] AlertServerTest add AlertPluginManager installPlugin

* [Fix-9221] [alert-server] replace @eventlistener with @PostConstruct

* [Fix-9221] [alert-server] sonar check solution

* [Improvement-9221] [alert] update constructor injection and replace IStoppable with Closeable

Co-authored-by: guoshupei <[email protected]>
lenboo pushed a commit that referenced this pull request Apr 11, 2022
@caishunfeng @devosend
@Tianqi-Dotes @Amy0104 @SbloodyS @EricJoy2048 @mans2singh @zhongjiajie @guoshupei @songjianet @EricGao888 @labbomb @huagetai @nobolity @zhuangchong @kezhenxu94
[cherry-pick-3.0.0-alpha] Cherry pick dev to 3.0.0 alpha (#9429)
9fda8c5 
* [Fix][UI Next][V1.0.0-Alpha]Add zh for dag execution policy (#9363)

* [Bug-9235][Alert]Fix wechat markdown message and change wechat form structure (#9367)

* fix wechat issues:
1. change table msg type to markdown.
2. change userId to not required and enrich hints
3. change 'app id' to 'app id and chat id'

* fix wechat issues:
1. revert table showtype and add markdown showtype.
2. enrich hints.
3. delete 'chatid', rename agentid to weChatAgentIdChatId.
4. modify code to send markdown message.

* fix wechat issues: Change the language pack of agentId to agentId/chatId.

* fix format

* fix param name

Co-authored-by: Amy <[email protected]>

* [FIX-9355] Fix scheduleTime of start-process-instance api in api-doc (#9359)

* fix #9355

* fix #9355

* fix ut error

* fix ut error

* [CI] try to fix ci (#9366)

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* try to fix ci

* [optimization] [Service] Optimization ProcessService and add ProcessService interface (#9370)

* [task-spark][docs] Corrected notice section (#9375)

* [python] Migrate pythonGatewayServer into api server (#9372)

Currently the size of our distribute package is up to
800MB, this patch is migrate python gateway server into
api server

The distribute package size before and after this patch is:

```sh
# before
796M   apache-dolphinscheduler-2.0.4-SNAPSHOT-bin.tar.gz

# after
647M   apache-dolphinscheduler-2.0.4-SNAPSHOT-bin.tar.gz
```

* [Fix][UI Next][V1.0.0-Alpha] Add light color theme to echarts. (#9381)

* [Bug][API-9364]fix ProcessInstance wrong alert group id (#9383)

* fix ProcessInstance wrong alert group id

* change  createComplementCommandList method to protected

* [BUG][WORKER-9349]fix param priority (#9379)

* fix param priority

* fix params priority code logic

* [Improvement] change method access (#9390)

* change method to protected

* change method access

* [Fix-9221] [alert-server] optimization and gracefully close (#9246)

* [Fix-9221] [alert-server] optimization and gracefully close

This closes #9221

* [Fix-9221] [alert-server] remove unused mock data

This closes #9221

* [Fix-9221] [alert-server] remove unused mock data

This closes #9221

* [Fix-9221] [alert-server] remove unnecessary Mockito stubbings

* [Fix-9221] [alert-server] init AlertPluginManager in AlertServer

* [Fix-9221] [alert-server] AlertServerTest add AlertPluginManager installPlugin

* [Fix-9221] [alert-server] replace @eventlistener with @PostConstruct

* [Fix-9221] [alert-server] sonar check solution

* [Improvement-9221] [alert] update constructor injection and replace IStoppable with Closeable

Co-authored-by: guoshupei <[email protected]>

* [Fix][UI Next][V1.0.0-Alpha] Fix the task instance forced success button multi-language support error. (#9392)

* [doc] Change get help from dev mail list to slack (#9377)

* Change all get help from dev mailing list to slack, because
  we find out mailing list have many users ask for subscribe
  and they maybe subscribe by accident.
* remove join dev mailing list in faq.md because we already
  have it in https://dolphinscheduler.apache.org/en-us/community/development/subscribe.html

* Add new code owner of docs module (#9388)

* [CI] Enable CI to remove unexpected files in /docs/img dir (#9393)

* [Bug][UI Next]Modify the display state logic of save buttons under workflow definition (#9403)

* Modifying site Configurations

* Modify the display state logic of save buttons under workflow definition

* [doc] Remove observability (#9402)

SkyWalking v9 is coming soon and there are without
DolphinScheduler menus anymore, So we should remove
the SW agent to avoid confusion.

close: #9242

* [DS-9387][refactor]Remove the lock in the start method of the MasterRegistryClient class (#9389)

* [Fix-9251] [WORKER] reslove the sql task about of add the udf resource failed (#9319)

* feat(resource  manager): extend s3 to the storage of ds

1.fix some spell question
2.extend the type of storage
3.add the s3utils
to manager resource
4.automatic inject the storage in addition to your
config

* fix(resource  manager): update the dependency

* fix(resource  manager): extend s3 to the storage of ds

fix the constant of hadooputils

* fix(resource  manager): extend s3 to the storage of ds

1.fix some spell question
2.delete the import *

* fix(resource  manager):

merge  the unitTest:
1.TenantServiceImpl
2.ResourceServiceImpl
3.UserServiceImpl

* fix(resource  manager): extend s3 to the storage of ds

merge the resourceServiceTest

* fix(resource  manager): test  cancel the test method

createTenant verifyTenant

* fix(resource  manager): merge the code  follow the check-result of sonar

* fix(resource  manager): extend s3 to the storage of ds

fit the spell question

* fix(resource  manager): extend s3 to the storage of ds

revert the common.properties

* fix(resource  manager): extend s3 to the storage of ds

update the storageConfig with None

* fix(resource  manager): extend s3 to the storage of ds

fix the judge of resourceType

* fix(resource  manager): extend s3 to the storage of ds

undo the compile-mysql

* fix(resource  manager): extend s3 to the storage of ds

delete hadoop aws

* fix(resource  manager): extend s3 to the storage of ds

update the know-dependencies to delete aws 1.7.4
update the e2e
file-manager common.properties

* fix(resource  manager): extend s3 to the storage of ds

update the aws-region

* fix(resource  manager): extend s3 to the storage of ds

fix the storageconfig init

* fix(resource  manager): update e2e docker-compose

update e2e docker-compose

* fix(resource  manager): extend s3 to the storage of ds

revent the e2e common.proprites

print the resource type in propertyUtil

* fix(resource  manager): extend s3 to the storage of ds
1.println the properties

* fix(resource  manager): println the s3 info

* fix(resource  manager): extend s3 to the storage of ds

delete the info  and upgrade the s3 info to e2e

* fix(resource  manager): extend s3 to the storage of ds

add the bucket init

* fix(resource  manager): extend s3 to the storage of ds

1.fix some spell question
2.delete the import *

* fix(resource  manager): extend s3 to the storage of ds

upgrade the s3 endpoint

* fix(resource  manager): withPathStyleAccessEnabled(true)

* fix(resource  manager): extend s3 to the storage of ds

1.fix some spell question
2.delete the import *

* fix(resource  manager): upgrade the  s3client builder

* fix(resource  manager): correct  the s3 point to s3client

* fix(resource  manager): update the constant BUCKET_NAME

* fix(resource  manager): e2e  s3 endpoint -> s3:9000

* fix(resource  manager): extend s3 to the storage of ds

1.fix some spell question
2.delete the import *

* style(resource  manager): add info to createBucket

* style(resource  manager): debug the log

* ci(resource  manager): test

test s3

* ci(ci): add INSERT INTO dolphinscheduler.t_ds_tenant (id, tenant_code, description, queue_id, create_time, update_time) VALUES(1, 'root', NULL, 1, NULL, NULL); to h2.sql

* fix(resource  manager): update the h2 sql

* fix(resource  manager): solve to delete the tenant

* style(resource  manager): merge the style end delete the unuse s3 config

* fix(resource  manager): extend s3 to the storage of ds

UPDATE the rename resources when s3

* fix(resource  manager): extend s3 to the storage of ds

1.fix the code style of QuartzImpl

* fix(resource  manager): extend s3 to the storage of ds

1.impoort restore_type to CommonUtils

* fix(resource  manager): update the work thread

* fix(resource  manager): update  the baseTaskProcessor

* fix(resource  manager): upgrade dolphinscheduler-standalone-server.xml

* fix(resource  manager): add  user Info to dolphinscheduler_h2.sql

* fix(resource  manager): merge  the resourceType to NONE

* style(upgrade the log level to info):

* fix(resource  manager): sysnc the h2.sql

* fix(resource  manager): update the merge the user tenant

* fix(resource  manager): merge the resourcesServiceImpl

* fix(resource  manager):

when the storage is s3 ,that the directory can't be renamed

* fix(resource  manager): in s3 ,the directory cannot be renamed

* fix(resource  manager): delete the deleteRenameDirectory in E2E

* fix(resource  manager): check the style and  recoverd the test

* fix(resource  manager): delete the log.print(LoginUser)

* fix(server): fix the  udf serialize

* fix(master  task): update the udfTest to update the json string

* fix(test): update the udfFuncTest

* fix(common): syn the common.properties

* fix(udfTest): upgrade the udfTest

* fix(common): revent the common.properties

* [Fix-9316] [Task] Configure DB2 data source SQL script execution report ResultSet has been closed exception in SQL task  (#9317)

* fix db2 error in the sql task

* update limit in sql task

* [UI] Migrate NPM to PNPM in CI builds (#9431)

Co-authored-by: Devosend <[email protected]>
Co-authored-by: Tq <[email protected]>
Co-authored-by: Amy <[email protected]>
Co-authored-by: xiangzihao <[email protected]>
Co-authored-by: gaojun2048 <[email protected]>
Co-authored-by: mans2singh <[email protected]>
Co-authored-by: Jiajie Zhong <[email protected]>
Co-authored-by: Amy0104 <[email protected]>
Co-authored-by: guoshupei <[email protected]>
Co-authored-by: guoshupei <[email protected]>
Co-authored-by: songjianet <[email protected]>
Co-authored-by: Eric Gao <[email protected]>
Co-authored-by: labbomb <[email protected]>
Co-authored-by: worry <[email protected]>
Co-authored-by: nobolity <[email protected]>
Co-authored-by: Kerwin <[email protected]>
Co-authored-by: kezhenxu94 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhuangchong zhuangchong zhuangchong approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Improvement] [alert-server] AlertSender optimization and gracefully close, such as MasterServer
4 participants
@guoshupei @codecov-commenter @caishunfeng @zhuangchong