Refactor LocalExecutorService and enhance testing coverage - #967
Open
qianmoQ wants to merge 17 commits into
Open
Refactor LocalExecutorService and enhance testing coverage#967qianmoQ wants to merge 17 commits into
qianmoQ wants to merge 17 commits into
Conversation
Pin current behaviour of LocalExecutorService before refactoring: streaming/legacy paths, pre-count, three cancel routes, NULL handling and column projection. Backed by a network-free FakePluginService so the suite runs without a real database. Two assertions intentionally capture known quirks (JDBC-abort cancel reporting count=0, and STOPPED state after all rows committed) to be fixed in a later phase.
Extract the 576-line class into focused units without behaviour change: - SyncStrategy + SyncContext with StreamingSyncStrategy / LegacySyncStrategy - TaskRegistry (object) for the running-task map and cancel executor, kept a singleton so stop() still works across service instances - TaskHandle / TaskCancelledException and ValueCodec for value/SQL codecs start() now only resolves inputs, runs pre-count and picks a strategy; stop() delegates to TaskRegistry. Characterization suite stays green and spotbugs reports no findings.
Unify cancellation behind a single CancellationToken and a runCancelable writer wrapper, collapsing the three former detection points (in-loop throw, post-loop flag, driver-abort exception) into one path that always surfaces TaskCancelledException with the committed count. Previously a stop that aborted the JDBC fetch reported count=0 because it read the stale rowsAtStop counter (updated only every 1000 rows); it now reports the rows actually committed to the target. Row counts are unified on writtenCount() across success and cancellation. Also precompute source-column indexes at onSchema instead of a per-row map lookup in the streaming path.
Add a per-executor timeout (seconds) to the local executor. A watchdog cancels the running sync once the limit is exceeded, surfacing it as RunState.TIMEOUT with the committed row count via the shared CancellationToken (USER stop vs TIMEOUT). The timeout defaults to 0 (disabled) in LocalExecutor's config, so existing syncs keep running to completion. DataSetServiceImpl now reads the value from each executor's effective config; executors without the field (Seatunnel) still fall back to 600s. TIMEOUT is treated as a terminal state like STOPPED so it is not overwritten to FAILURE.
Every plugin is reached through the datacap JDBC-conversion driver (type() defaults to JDBC, supportsStreaming()==true) and the sync target is always the JDBC dataset store, so the both-ends-non-streaming branch that hand-built INSERT strings was unreachable in practice. Remove that branch along with ValueCodec.formatSqlLiteral / escapeSqlString (manual quoting was an injection and type-coercion hazard, and carried a stray NUL byte in a comment). The legacy path now requires a batch-write-capable target and fails fast with a clear message otherwise; the streaming path is unaffected.
Drive LocalExecutorService against a real PostgreSQL container (source and
target) through a real-JDBC PluginService, verifying an actual source->target
sync: streaming copy with column projection/rename, NULL and numeric handling,
pre-count totals, and a real Statement.cancel() stop.
Pin the docker-java API version (1.41) in-code so Testcontainers negotiates
with newer Docker Desktop engines, and skip gracefully when Docker is absent.
Add spotbugs suppressions on the test helpers for false positives (Kotlin
use{} obligations, list casts, test data exposure).
Extract SeaTunnelCommander construction from SeatunnelExecutorService.start() into a package-visible buildCommander(request) (no behaviour change) and add tests asserting the resulting command line for the Spark and SeaTunnel engines. This locks the ExecutorRequest-to-command mapping so the upcoming ExecutorRequest restructure can be verified against a stable expected command.
Collapse the executor-specific typed fields (executorHome, startScript, runWay, runMode, runEngine, fetchSize, batchSize, preCount) into a single options string map, leaving only executor-agnostic fields on ExecutorRequest (taskName, userName, input, output, workHome, timeout, transform, progressListener, pluginManager). Reduce the six telescoping constructors to the primary plus one workHome convenience constructor. Each executor now reads what it needs from options: the local executor takes fetchSize/batchSize/preCount, seatunnel takes home/startScript/way/mode/engine. Callers pass the executor's effective config map straight through as options. Adding a new executor no longer requires touching this shared class. Behaviour is preserved: local characterization + PostgreSQL E2E tests stay green, and the seatunnel command-mapping test asserts the same command line.
Introduce ant-design-vue 4 and wire it globally next to the existing view-shadcn-ui so components can be migrated page by page without breaking untouched pages. Add composables for cross-cutting concerns: - useTheme: single-source dark/light switch (persisted, toggles the html `dark` class and drives antd's dark/default algorithm) - useAntdLocale: maps the vue-i18n locale to antd's built-in locale App.vue now wraps the router view in a-config-provider (theme + locale). No reset.css yet to avoid affecting not-yet-migrated pages. Pin pnpm onlyBuiltDependencies to keep pnpm 10 CI from failing on ignored builds.
Replace ShadcnSelect with a-select (grouped options) and drop the view-shadcn-ui setLocale call; antd's built-in strings now follow the vue-i18n locale through a-config-provider. loadLocale already updates the global locale, so switching language reactively updates both app and antd text.
Replace ShadcnException with a-result (status 404/403/500, title/sub-title, extra + icon slots) across NotFound, NotAuthorized and NotNetwork. Swap ShadcnButton to a-button and its `to` navigation to router pushes on click; NotNetwork's custom icon now uses antd's WarningOutlined.
Rebuild the sign-in form on antd: a-form (:model + @finish/@finishFailed) with a-form-item rules, a-input / a-input-password, submit via html-type=submit, and a-card / a-avatar / a-divider / a-space / a-spin. Global $Message calls become antd message; validation-error reporting adapts to antd's errorFields shape. Field rules and behaviour are unchanged.
Mirror the sign-in migration for sign-up: antd a-form with a-input / a-input-password, submit via html-type=submit, a-card / a-avatar / a-divider / a-space / a-spin, and antd message. The confirm-password custom validator now uses antd's (rule, value) signature; rules and behaviour unchanged.
First list-page migration proving the hard patterns: - ShadcnTable -> a-table with #bodyCell slot (role tags, action buttons); useHeaders now emits antd columns (title/dataIndex/key) - ShadcnPagination -> a-pagination (v-model:current, show-size-changer, change / show-size-change events) - ShadcnSpin -> a-spin wrapper (:spinning); a-card / a-tag / a-tooltip / a-button / a-space Also convert the page from Options API to <script setup> composition style (refs + functions + onMounted). Icons keep ShadcnIcon for now (migrated in a later dedicated pass). UserRole / UserInfo dialogs untouched (coexistence).
Convert NotFound / NotAuthorized / NotNetwork from Options API to <script setup> (defineOptions for name), completing their composition-style refactor alongside the earlier ant-design-vue swap. Templates unchanged.
Convert AuthSignin / AuthSignup from Options API to <script setup> (refs + functions, useI18n for translations, defineOptions for name), finishing the composition-style refactor of the already-migrated pages. Templates and behaviour unchanged.
…n API Proves the modal pattern: ShadcnModal -> a-modal (v-model:open, footer=null with the form's own submit button); ShadcnCheckboxGroup/Checkbox -> a-checkbox-group / a-checkbox; a-spin wrapper. Convert to <script setup> (defineProps/withDefaults, defineEmits, computed visible get/set, useI18n, antd message). Behaviour and validation unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one)
Changelog entry (Details of this change)
Affected version
latest version