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

[CDAP-21096] Keep TaskWorkerServiceLauncher, SystemWorkerServiceLauncher, CoreSchedulerService, Schedule HTTP handlers in Appfabric Processor #15844

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

vsethi09
Copy link
Contributor

@vsethi09 vsethi09 commented Jan 31, 2025

CDAP-21096 Keep TaskWorkerServiceLauncher, SystemWorkerServiceLauncher, CoreSchedulerService, Schedule HTTP handlers in Appfabric Processor (singleton service)

Change Description

  • Removed TaskWorkerServiceLauncher and SystemWorkerServiceLauncher from AppFabricServiceMain.
  • Removed CoreSchedulerService to Appfabric server.
  • Moved ScheduleNotificationSubscriberService to CoreSchedulerService as it used to be before [CDAP-21096] Split Appfabric into stateless service and stateful processor #15773.
  • Moved Program Scheduled related handlers from ProgramLifecycleHttpHandler to a new handler ProgramScheduleHttpHandler, which runs only in Appfabric Processor.

Verification

  • Unit Tests
  • CDAP sandbox (tested with 100 scheduled pipelines runs)
  • Distributed CDAP on k8s (tested with 100 scheduled pipelines runs)

@vsethi09 vsethi09 added the build Triggers github actions build label Jan 31, 2025
@vsethi09 vsethi09 force-pushed the fix/CDAP-21096_task_workers branch 6 times, most recently from e806137 to 562c886 Compare February 4, 2025 14:38
@vsethi09 vsethi09 changed the title [WIP][CDAP-21096] Skip launching of task workers and system worker from Appfabric service [CDAP-21096] Keep TaskWorkerServiceLauncher, SystemWorkerServiceLauncher, CoreSchedulerService, Schedule HTTP handlers in Appfabric Processor Feb 4, 2025
@vsethi09 vsethi09 marked this pull request as ready for review February 4, 2025 14:44
}

/**
* Returns status of a type specified by the type{flows,workflows,mapreduce,spark,services,schedules}.
Copy link
Member

Choose a reason for hiding this comment

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

nit: returns status of schedule.

similar comment for all such comments. Please also update them in ProgramLifecycleHttpHandler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

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

couple of nits, otherwise LGTM

@vsethi09 vsethi09 force-pushed the fix/CDAP-21096_task_workers branch 2 times, most recently from 7b5a0cd to 4e6f294 Compare February 4, 2025 17:58
Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

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

Please fix checkstyle issues in the files modified in the PR.

…her, CoreSchedulerService, Schedule HTTP handlers in Appfabric Processor

Move scheduler services and http handler to appfabric processor

Add max retry count in Provisioning service
@vsethi09 vsethi09 force-pushed the fix/CDAP-21096_task_workers branch from 4e6f294 to bd35cab Compare February 5, 2025 07:36
Copy link

sonarqubecloud bot commented Feb 5, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
79.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@vsethi09 vsethi09 merged commit bd09667 into develop Feb 5, 2025
9 of 10 checks passed
@vsethi09 vsethi09 deleted the fix/CDAP-21096_task_workers branch February 5, 2025 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Triggers github actions build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants