From 05efe36bc2ed64d7d6fd02c20e3bfd58e8efe178 Mon Sep 17 00:00:00 2001 From: George Date: Tue, 25 Jun 2024 11:49:36 +0000 Subject: [PATCH 01/18] Enable playwright tests Signed-off-by: George --- .arg | 2 +- .gitignore | 12 ++++--- Earthfile | 31 ++++++++++++++----- docs/contributors/ui-testing.md | 29 ++++++++++------- web-console/.github/workflows/playwright.yml | 2 +- web-console/package.json | 4 +-- web-console/playwright-ct.config.ts | 5 +-- ...ght.config.ts => playwright-e2e.config.ts} | 10 +++--- .../(app)/streaming/builder/page.tsx | 7 ++++- .../src/lib/components/common/JSONEditor.tsx | 5 ++- .../dialogs/DeltaLakeInputConnector.spec.tsx | 2 +- .../components/layouts/analytics/editor.tsx | 10 +++++- .../management/PipelineResourcesDialog.tsx | 9 ++++-- .../management/PipelineResourcesThumb.tsx | 2 +- .../src/lib/functions/navigation/vertical.tsx | 2 +- web-console/tests/e2e/demoAccrual.spec.ts | 24 +++++++++++--- web-console/tests/e2e/entityRename.spec.ts | 17 +++------- .../tests/e2e/felderaBasicsTutorial.spec.ts | 2 +- .../global.setup-e2e.ts} | 16 +++++----- 19 files changed, 126 insertions(+), 65 deletions(-) rename web-console/{playwright.config.ts => playwright-e2e.config.ts} (87%) rename web-console/tests/{global.setup.ts => e2e/global.setup-e2e.ts} (64%) diff --git a/.arg b/.arg index bd67345cda..8534f29ba4 100644 --- a/.arg +++ b/.arg @@ -1,2 +1,2 @@ # File used by earthly -PLAYWRIGHT_SNAPSHOTS_COMMIT=6d7a9e13a28a4a95a92ef5ca3acba864ed399ee5 +PLAYWRIGHT_SNAPSHOTS_COMMIT=7e91647f7b85cfc472c61271d6115c566851b76c diff --git a/.gitignore b/.gitignore index 7b75c76283..4fb21a8ff6 100644 --- a/.gitignore +++ b/.gitignore @@ -59,10 +59,14 @@ gh-pages # Playwright playwright-artifacts/ playwright-snapshots/ -/test-results/ -/playwright-report/ -/blob-report/ -/playwright/.cache/ +test-results/ +playwright-report/ +test-results-ct/ +playwright-report-ct/ +test-results-e2e/ +playwright-report-e2e/ +blob-report/ +playwright/.cache/ # Local cargo configuration. /.cargo/config.toml diff --git a/Earthfile b/Earthfile index 865cb1aa4f..a9a1f03cdf 100644 --- a/Earthfile +++ b/Earthfile @@ -512,7 +512,7 @@ ui-playwright-container: # Install zip to prepare test artifacts for export RUN apt-get install -y zip -ui-playwright-tests: +ui-playwright-tests-e2e: FROM +ui-playwright-container ENV FELDERA_VERSION=latest @@ -521,15 +521,31 @@ ui-playwright-tests: --compose ../docker-compose.yml \ --service pipeline-manager # We zip artifacts regardless of test success or error, and then we complete the command preserving test's exit_code - RUN if yarn playwright test; then exit_code=0; else exit_code=$?; fi \ + RUN sleep 10 && if yarn playwright test -c playwright-e2e.config.ts; then exit_code=0; else exit_code=$?; fi \ && cd /dbsp \ - && zip -r playwright-report.zip playwright-report \ - && zip -r test-results.zip test-results \ + && zip -r playwright-report-e2e.zip playwright-report-e2e \ + && zip -r test-results-e2e.zip test-results-e2e \ && exit $exit_code END FINALLY - SAVE ARTIFACT --if-exists /dbsp/playwright-report.zip AS LOCAL ./playwright-artifacts/ - SAVE ARTIFACT --if-exists /dbsp/test-results.zip AS LOCAL ./playwright-artifacts/ + SAVE ARTIFACT --if-exists /dbsp/playwright-report-e2e.zip AS LOCAL ./playwright-artifacts/ + SAVE ARTIFACT --if-exists /dbsp/test-results-e2e.zip AS LOCAL ./playwright-artifacts/ + END + +ui-playwright-tests-ct: + FROM +ui-playwright-container + ENV FELDERA_VERSION=latest + + TRY + # We zip artifacts regardless of test success or error, and then we complete the command preserving test's exit_code + RUN if yarn playwright test -c playwright-ct.config.ts; then exit_code=0; else exit_code=$?; fi \ + && cd /dbsp \ + && zip -r playwright-report-ct.zip playwright-report-ct \ + && zip -r test-results-ct.zip test-results-ct \ + && exit $exit_code + FINALLY + SAVE ARTIFACT --if-exists /dbsp/playwright-report-ct.zip AS LOCAL ./playwright-artifacts/ + SAVE ARTIFACT --if-exists /dbsp/test-results-ct.zip AS LOCAL ./playwright-artifacts/ END benchmark: @@ -573,7 +589,8 @@ all-tests: BUILD +openapi-checker BUILD +test-sql BUILD +integration-tests - # BUILD +ui-playwright-tests + BUILD +ui-playwright-tests-ct + BUILD +ui-playwright-tests-e2e BUILD +test-docker-compose # BUILD +test-docker-compose-stable BUILD +test-debezium-mysql diff --git a/docs/contributors/ui-testing.md b/docs/contributors/ui-testing.md index 75c3dedb26..dc7d3b68d7 100644 --- a/docs/contributors/ui-testing.md +++ b/docs/contributors/ui-testing.md @@ -1,19 +1,22 @@ # Web Console testing -Regression UI testing is performed in the form of end-to-end integration tests using Playwright framework. - -### e2e testing with Playwright -Existing Playwright tests are executed during CI -and can be run manually within provided devcontainer environment. +Regression UI testing is performed using Playwright framework. +The repository contains end-to-end (e2e) tests that are to be executed against a clean Feldera instance, +and standalone unit (ct) tests that include UI component tests and TypeScript function tests. +Existing Playwright tests are executed during CI and can be run manually within provided devcontainer environment. +Add environment variable `CI=true` when executing tests in CI setting. -### Running tests +### Running e2e tests -Run `yarn test` -to execute all tests on all supported platforms in background, or run `yarn test:ui` to open a UI to run tests interactively. -Add environment variable `CI=true` when executing tests in CI setting. +Run `yarn test-e2e` to execute all e2e tests on all supported platforms in background, or run `yarn test-e2e-ui` to open a UI to run tests interactively. Tests should be executed against a running Pipeline Manager instance. As an artificial limitation of scope, currently no services for Kafka, Debezium, Snowflake and other similar connector types are available for tests in the CI, so only HTTP connectors and API is available along with the UI itself. +### Running ct tests + +Run `yarn test-ct` to execute all ct tests on all supported platforms in background, or run `yarn test-ct-ui` to open a UI to run tests interactively. +Unit tests do not need Feldera instance to run because they run against individual ESM modules compiled on-demand for the test. + ### Contributing tests The tests directory is `feldera/web-console/tests`. @@ -25,15 +28,19 @@ Environment variable `PLAYWRIGHT_SNAPSHOTS_COMMIT` in `feldera/.arg` specifies t When committing new tests or updating screenshots for existing tests, `PLAYWRIGHT_SNAPSHOTS_COMMIT` needs to be updated as well. When testing locally, you need to manually clone `playwright-snapshots` and checkout the correct commit hash, e.g.: +``` +cd web-console && yarn test-prepare +``` +OR ``` cd web-console npm i -g degit -degit feldera/playwright-snapshots#2adf778 +degit feldera/playwright-snapshots#{commit_hash} ``` OR ``` cd web-console -git clone https://github.com/feldera/playwright-snapshots && git checkout 2adf778 +git clone https://github.com/feldera/playwright-snapshots && git checkout {commit_hash} ``` diff --git a/web-console/.github/workflows/playwright.yml b/web-console/.github/workflows/playwright.yml index 3ff931540f..3a85df483e 100644 --- a/web-console/.github/workflows/playwright.yml +++ b/web-console/.github/workflows/playwright.yml @@ -18,7 +18,7 @@ jobs: - name: Install Playwright Browsers run: yarn playwright install --with-deps - name: Run Playwright tests - run: yarn playwright test + run: yarn playwright test -c playwright-ct.config.ts && yarn playwright test -c playwright-e2e.config.ts - uses: actions/upload-artifact@v4 if: always() with: diff --git a/web-console/package.json b/web-console/package.json index 06d303e3e7..8a41079851 100644 --- a/web-console/package.json +++ b/web-console/package.json @@ -24,8 +24,8 @@ "format-check": "prettier --check \"src/**/*.{js,jsx,ts,tsx}\"", "generate-openapi": "openapi --input ../openapi.json --request ./src/lib/services/manager/customRequest.ts --output ./src/lib/services/manager && yarn format && patch -p2 < openapi-fixes.patch", "build-openapi": "cd .. && cargo run --bin pipeline-manager -- --dump-openapi", - "test": "PLAYWRIGHT_API_ORIGIN=http://localhost:8080/ PLAYWRIGHT_APP_ORIGIN=http://localhost:8080/ DISPLAY= yarn playwright test", - "test-ui": "PLAYWRIGHT_API_ORIGIN=http://localhost:8080/ PLAYWRIGHT_APP_ORIGIN=http://localhost:8080/ DISPLAY= yarn playwright test --ui-host=0.0.0.0", + "test-e2e": "PLAYWRIGHT_API_ORIGIN=http://localhost:8080/ PLAYWRIGHT_APP_ORIGIN=http://localhost:8080/ DISPLAY= yarn playwright test -c playwright-e2e.config.ts", + "test-e2e-ui": "PLAYWRIGHT_API_ORIGIN=http://localhost:8080/ PLAYWRIGHT_APP_ORIGIN=http://localhost:8080/ DISPLAY= yarn playwright test -c playwright-e2e.config.ts --ui-host=0.0.0.0", "test-ct": "DISPLAY= yarn playwright test -c playwright-ct.config.ts", "test-ct-ui": "DISPLAY= yarn playwright test -c playwright-ct.config.ts --ui-host=0.0.0.0", "test-report": "yarn playwright show-report", diff --git a/web-console/playwright-ct.config.ts b/web-console/playwright-ct.config.ts index 80f14727f3..3b9db96f32 100644 --- a/web-console/playwright-ct.config.ts +++ b/web-console/playwright-ct.config.ts @@ -11,8 +11,9 @@ export default defineConfig({ testDir: './src/lib', /* The base directory, relative to the config file, for snapshot files created with toMatchSnapshot and toHaveScreenshot. */ snapshotDir: './playwright-snapshots/ct', + outputDir: 'test-results-ct', /* Maximum time one test can run for. */ - timeout: 5 * 1000, + timeout: 10 * 1000, /* Run tests in files in parallel */ fullyParallel: true, /* Fail the build on CI if you accidentally left test.only in the source code. */ @@ -22,7 +23,7 @@ export default defineConfig({ /* Opt out of parallel tests on CI. */ workers: process.env.CI ? 1 : undefined, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'html', + reporter: [['html', { outputFolder: 'playwright-report-ct' }]], /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ diff --git a/web-console/playwright.config.ts b/web-console/playwright-e2e.config.ts similarity index 87% rename from web-console/playwright.config.ts rename to web-console/playwright-e2e.config.ts index f9ae4e21e6..1b1fc1a55a 100644 --- a/web-console/playwright.config.ts +++ b/web-console/playwright-e2e.config.ts @@ -13,7 +13,10 @@ export const appOrigin = process.env.PLAYWRIGHT_APP_ORIGIN! * See https://playwright.dev/docs/test-configuration. */ export default defineConfig({ - testDir: './tests', + testDir: './tests/e2e', + /* The base directory, relative to the config file, for snapshot files created with toMatchSnapshot and toHaveScreenshot. */ + snapshotDir: 'playwright-snapshots/e2e', + outputDir: 'test-results-e2e', /* Run tests in files in parallel */ fullyParallel: true, /* Fail the build on CI if you accidentally left test.only in the source code. */ @@ -23,8 +26,7 @@ export default defineConfig({ /* Opt out of parallel tests on CI. */ workers: process.env.CI ? 1 : 1 /* undefined */, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: 'html', - snapshotDir: 'playwright-snapshots', + reporter: [['html', { outputFolder: 'playwright-report-e2e' }]], /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Base URL to use in actions like `await page.goto('/')`. */ @@ -39,7 +41,7 @@ export default defineConfig({ projects: [ { name: 'setup', - testMatch: /global\.setup\.ts/, + testMatch: /global\.setup-e2e\.ts/, }, { name: 'chromium', diff --git a/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/builder/page.tsx b/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/builder/page.tsx index a16f27eb75..9074abc0e6 100644 --- a/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/builder/page.tsx +++ b/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/builder/page.tsx @@ -303,7 +303,12 @@ const PipelineBuilderPage = ({ )} {pipeline.name && ( - )} diff --git a/web-console/src/lib/components/common/JSONEditor.tsx b/web-console/src/lib/components/common/JSONEditor.tsx index 38890cb81c..dd114a90cb 100644 --- a/web-console/src/lib/components/common/JSONEditor.tsx +++ b/web-console/src/lib/components/common/JSONEditor.tsx @@ -139,8 +139,11 @@ export function JSONEditor(props: { defaultLanguage='yaml' options={{ ...isMonacoEditorDisabled(props.disabled), + overviewRulerLanes: 0, + hideCursorInOverviewRuler: true, + overviewRulerBorder: false, scrollbar: { - vertical: 'hidden' + vertical: 'visible' } }} value={props.valueToText(props.value)} diff --git a/web-console/src/lib/components/connectors/dialogs/DeltaLakeInputConnector.spec.tsx b/web-console/src/lib/components/connectors/dialogs/DeltaLakeInputConnector.spec.tsx index 9f4ce394c6..5686c65a9e 100644 --- a/web-console/src/lib/components/connectors/dialogs/DeltaLakeInputConnector.spec.tsx +++ b/web-console/src/lib/components/connectors/dialogs/DeltaLakeInputConnector.spec.tsx @@ -10,7 +10,7 @@ import { DeltaLakeInputConnectorDialog } from './DeltaLakeInputConnector' test.use({ viewport: { width: 1000, height: 800 } }) test('DeltaLake input creation', async ({ mount, page }) => { - test.setTimeout(20000) + test.setTimeout(40000) let config: any = null await mount( diff --git a/web-console/src/lib/components/layouts/analytics/editor.tsx b/web-console/src/lib/components/layouts/analytics/editor.tsx index af48b98433..d200c4c801 100644 --- a/web-console/src/lib/components/layouts/analytics/editor.tsx +++ b/web-console/src/lib/components/layouts/analytics/editor.tsx @@ -295,7 +295,15 @@ export const ProgramEditorImpl = ({ wrapperProps={{ 'data-testid': 'box-program-code-wrapper' }} - options={isMonacoEditorDisabled(status === 'isLoading')} + options={{ + ...isMonacoEditorDisabled(status === 'isLoading'), + overviewRulerLanes: 0, + hideCursorInOverviewRuler: true, + overviewRulerBorder: false, + scrollbar: { + vertical: 'visible' + } + }} /> diff --git a/web-console/src/lib/components/streaming/management/PipelineResourcesDialog.tsx b/web-console/src/lib/components/streaming/management/PipelineResourcesDialog.tsx index 0e986604e2..c54d6bfa19 100644 --- a/web-console/src/lib/components/streaming/management/PipelineResourcesDialog.tsx +++ b/web-console/src/lib/components/streaming/management/PipelineResourcesDialog.tsx @@ -245,7 +245,12 @@ export const PipelineResourcesForm = (props: { disabled?: boolean }) => { - + @@ -311,7 +316,7 @@ export const PipelineResourcesForm = (props: { disabled?: boolean }) => { For running pipelines, changes only take effect upon restart. - diff --git a/web-console/src/lib/components/streaming/management/PipelineResourcesThumb.tsx b/web-console/src/lib/components/streaming/management/PipelineResourcesThumb.tsx index 2d54d9dc0c..32d441f727 100644 --- a/web-console/src/lib/components/streaming/management/PipelineResourcesThumb.tsx +++ b/web-console/src/lib/components/streaming/management/PipelineResourcesThumb.tsx @@ -14,7 +14,7 @@ export const PipelineResourcesThumb = (props: { pipelineName: string }) => { } const res = configQuery.data.resources return ( - + {configQuery.data.workers} workers |  CPU: {printRange(res?.cpu_cores_min, res?.cpu_cores_max, 'cores', 'default')} |  diff --git a/web-console/src/lib/functions/navigation/vertical.tsx b/web-console/src/lib/functions/navigation/vertical.tsx index 534bf2c592..716a767631 100644 --- a/web-console/src/lib/functions/navigation/vertical.tsx +++ b/web-console/src/lib/functions/navigation/vertical.tsx @@ -86,7 +86,7 @@ const navigation = (props: { showSettings: boolean }): VerticalNavItemsType => { openInNewTab: true, testid: 'button-vertical-nav-slack' } - ].flat() as VerticalNavItemsType + ].flat(2) as VerticalNavItemsType } export default navigation diff --git a/web-console/tests/e2e/demoAccrual.spec.ts b/web-console/tests/e2e/demoAccrual.spec.ts index eb18ffca2d..4cc5889adf 100644 --- a/web-console/tests/e2e/demoAccrual.spec.ts +++ b/web-console/tests/e2e/demoAccrual.spec.ts @@ -3,7 +3,7 @@ import invariant from 'tiny-invariant' import { faker } from '@faker-js/faker' import { expect, test } from '@playwright/test' -import { apiOrigin, appOrigin } from '../../playwright.config' +import { apiOrigin, appOrigin } from '../../playwright-e2e.config' import { deletePipeline, deleteProgram } from '../util' import demoAccrualSql from './demoAccrual.sql' @@ -18,6 +18,7 @@ test('Accrual demo test', async ({ page, request }) => { await page.getByTestId('button-vertical-nav-sql-programs').click() await page.getByTestId('button-add-sql-program').first().click() await page.getByTestId('input-program-name').fill(programName) + await page.getByTestId('box-save-saved').waitFor() await page.getByTestId('box-program-code-wrapper').getByRole('textbox').waitFor({ state: 'attached' }) await page.getByTestId('box-program-code-wrapper').getByRole('textbox').fill(demoAccrualSql) await page.getByTestId('box-program-code-wrapper').getByRole('textbox').blur() @@ -35,6 +36,19 @@ test('Accrual demo test', async ({ page, request }) => { await page.getByTestId('box-builder-program-options').getByRole('option', { name: programName }).click() await page.getByTestId('box-save-saved').waitFor() + // Ensure storage is disabled + await page.getByTestId('button-configure-resources').click() + if (await page.getByTestId('input-enable-storage').inputValue() === 'false') { + // Double switch to force `false` value from an undefined state + await page.waitForTimeout(200) + await page.getByTestId('input-enable-storage').click() + } + await page.waitForTimeout(200) + await page.getByTestId('input-enable-storage').click() + await page.waitForTimeout(200) + await expect(page).toHaveScreenshot('2-1.png') + await page.getByTestId('button-apply').click() + await page.getByTestId('button-breadcrumb-pipelines').click() await page.getByTestId(`box-pipeline-actions-${pipelineName}`).waitFor() await page.getByTestId(`box-pipeline-actions-${pipelineName}`).getByTestId('button-edit').click() @@ -45,6 +59,7 @@ test('Accrual demo test', async ({ page, request }) => { }) await test.step('Start the pipeline', async () => { + await page.getByTestId('box-resources-thumb').waitFor() await page.getByTestId('button-breadcrumb-pipelines').click() await page.getByTestId(`box-pipeline-actions-${pipelineName}`).waitFor() await expect(page).toHaveScreenshot('3-1-compiling-program-binary.png') @@ -199,8 +214,9 @@ test('Accrual demo test', async ({ page, request }) => { await page.getByTestId('box-relation-options').getByTestId(`button-option-relation-${relation}`).click() await page.getByTestId('button-expand-relations').hover() // Prevent the cursor causing flakes by tooltip popups await page.getByTestId('box-relation-options').waitFor({ state: 'hidden' }) - await page.getByTestId('box-relation-row').first().waitFor() - await page.waitForTimeout(50) + await page.getByTestId('box-relation-row').nth(2).waitFor() + await page.waitForLoadState('domcontentloaded') + await page.waitForTimeout(1000) await expect(page).toHaveScreenshot(`5-1-relation ${relation}.png`) } }) @@ -249,4 +265,4 @@ const inBatches = async ( result.push(...data) } return result -} +} \ No newline at end of file diff --git a/web-console/tests/e2e/entityRename.spec.ts b/web-console/tests/e2e/entityRename.spec.ts index 5e4c6596b6..8a10c8cf3d 100644 --- a/web-console/tests/e2e/entityRename.spec.ts +++ b/web-console/tests/e2e/entityRename.spec.ts @@ -1,15 +1,9 @@ -import invariant from 'tiny-invariant' - -import { faker } from '@faker-js/faker' import { expect, test } from '@playwright/test' -import { apiOrigin, appOrigin } from '../../playwright.config' +import { appOrigin } from '../../playwright-e2e.config' import { deleteConnectors, deletePipeline, deleteProgram } from '../util' -import demoAccrualSql from './demoAccrual.sql' import felderaBasicsTutorialSql from './felderaBasicsTutorial.sql' -const codeEditorScrollbarFadeTimeout = 1000 - /** * The usage of API entity names as identifiers introduces more complex UI states when editing the names. * This test performs renames of program, connector and pipeline entities in various circumstances, @@ -28,14 +22,12 @@ test('Entity rename test', async ({ page, request }) => { await page.getByTestId('box-program-code-wrapper').getByRole('textbox').waitFor({ state: 'attached' }) await page.getByTestId('box-program-code-wrapper').getByRole('textbox').fill(felderaBasicsTutorialSql) await page.getByTestId('box-program-code-wrapper').getByRole('textbox').blur() - await page.waitForTimeout(codeEditorScrollbarFadeTimeout) await expect(page).toHaveScreenshot('1-2-create-program1.png', { mask: ['box-spinner'].map(id => page.getByTestId(id)) }) await page.getByTestId('input-program-name').fill('program1_1') await page.getByTestId('box-save-saved').waitFor() - await page.waitForTimeout(codeEditorScrollbarFadeTimeout) await page.getByTestId('box-compile-status-success').waitFor() await expect(page).toHaveScreenshot('1-3-create-program1.png', { mask: ['box-spinner'].map(id => page.getByTestId(id)) @@ -44,7 +36,6 @@ test('Entity rename test', async ({ page, request }) => { await page.getByTestId('input-program-name').fill('program1_2') await page.getByTestId('box-save-saved').waitFor() await page.getByTestId('box-compile-status-success').waitFor() - await page.waitForTimeout(codeEditorScrollbarFadeTimeout) await expect(page).toHaveScreenshot('1-4-create-program1.png', { mask: ['box-spinner'].map(id => page.getByTestId(id)) }) @@ -61,15 +52,14 @@ test('Entity rename test', async ({ page, request }) => { await page.getByTestId('button-add-sql-program').first().click() await page.getByTestId('input-program-name').fill('program1_2') - await expect(page).toHaveScreenshot('2-1-saved-program2.png') + await expect(page).toHaveScreenshot('2-1-saved-program2.png') await page.getByTestId('box-program-code-wrapper').getByRole('textbox').waitFor({ state: 'attached' }) await page.getByTestId('box-program-code-wrapper').getByRole('textbox').fill(felderaBasicsTutorialSql) await page.getByTestId('box-program-code-wrapper').getByRole('textbox').blur() await page.getByTestId('input-program-name').fill('program2_0') await page.getByTestId('box-save-saved').waitFor() - await page.waitForTimeout(codeEditorScrollbarFadeTimeout) await expect(page).toHaveScreenshot('2-2-saved-program2.png', { mask: ['box-spinner'].map(id => page.getByTestId(id)) }) @@ -88,7 +78,6 @@ test('Entity rename test', async ({ page, request }) => { await page.getByTestId('input-program-name').fill('program2_1') await page.getByTestId('box-save-saved').waitFor() - await page.waitForTimeout(codeEditorScrollbarFadeTimeout) await expect(page).toHaveScreenshot('2-5-saved-program2.png', { mask: ['box-spinner'].map(id => page.getByTestId(id)) }) @@ -238,6 +227,7 @@ test('Entity rename test', async ({ page, request }) => { await page.getByTestId('input-topic').fill('preferred_vendor') await page.getByTestId('button-tab-auth').click() await page.getByTestId('button-next').click() + await page.getByTestId('button-next').click() await page.getByTestId('button-create').click() await page .getByTestId('box-handle-output-' + 'preferred_vendor-redpanda') @@ -263,6 +253,7 @@ test('Entity rename test', async ({ page, request }) => { await page.getByTestId('button-confirm-delete').click() await expect(page).toHaveScreenshot('a-4-edit-pipeline2.png') await page.getByTestId('input-pipeline-name').fill('pipeline2') + await page.getByTestId('box-error-name').waitFor({state: 'detached'}) await expect(page).toHaveScreenshot('a-5-edit-pipeline2.png') await page.getByTestId('box-save-saved').waitFor() await expect(page).toHaveScreenshot('a-6-edit-pipeline2.png') diff --git a/web-console/tests/e2e/felderaBasicsTutorial.spec.ts b/web-console/tests/e2e/felderaBasicsTutorial.spec.ts index 41e77c8c43..4eb89439b6 100644 --- a/web-console/tests/e2e/felderaBasicsTutorial.spec.ts +++ b/web-console/tests/e2e/felderaBasicsTutorial.spec.ts @@ -2,7 +2,7 @@ import invariant from 'tiny-invariant' import { expect, Page, test } from '@playwright/test' -import { apiOrigin, appOrigin } from '../../playwright.config' +import { apiOrigin, appOrigin } from '../../playwright-e2e.config' import { deleteConnectors, deletePipeline, deleteProgram } from '../util' import felderaBasicsTutorialSql from './felderaBasicsTutorial.sql' diff --git a/web-console/tests/global.setup.ts b/web-console/tests/e2e/global.setup-e2e.ts similarity index 64% rename from web-console/tests/global.setup.ts rename to web-console/tests/e2e/global.setup-e2e.ts index b2d6b75184..e2d6a40b2f 100644 --- a/web-console/tests/global.setup.ts +++ b/web-console/tests/e2e/global.setup-e2e.ts @@ -1,20 +1,20 @@ import { Page, test as setup } from '@playwright/test' -import { appOrigin } from '../playwright.config' +import { appOrigin } from '../../playwright-e2e.config' -const deleteRows = async (page: Page, regex: RegExp) => { +const rowsAction = async (page: Page, regex: RegExp, actionTestId: string, postProcedure = async () => {}) => { while (true) { try { // Wait for at least one row, if timed out - no rows left await page.getByTestId(regex).waitFor({ timeout: 2000 }) } catch {} - const buttonDelete = await page.getByTestId(regex).first().getByTestId('button-delete') + const buttonDelete = await page.getByTestId(regex).first().getByTestId(actionTestId) if (!(await buttonDelete.isVisible())) { // Exit if no more rows left break } await buttonDelete.click() - await page.getByTestId('button-confirm-delete').click() + await postProcedure() } } @@ -34,18 +34,20 @@ setup('Global prepare', async ({ page, request }) => { await setup.step('Prepare: Delete pipelines', async () => { await page.getByTestId('button-vertical-nav-pipelines').click() - await deleteRows(page, /box-pipeline-actions-/) + await rowsAction(page, /box-pipeline-actions-/, 'button-shutdown') + await page.waitForTimeout(4000) + await rowsAction(page, /box-pipeline-actions-/, 'button-delete', () => page.getByTestId('button-confirm-delete').click()) }) await setup.step('Prepare: Delete connectors', async () => { await page.getByTestId('button-vertical-nav-connectors').click() - await deleteRows(page, /box-connector-actions-/) + await rowsAction(page, /box-connector-actions-/, 'button-delete', () => page.getByTestId('button-confirm-delete').click()) }) await setup.step('Prepare: Delete programs', async () => { await page.getByTestId('button-vertical-nav-sql-programs').click() - await deleteRows(page, /box-program-actions-/) + await rowsAction(page, /box-program-actions-/, 'button-delete', () => page.getByTestId('button-confirm-delete').click()) }) }) From e542d9ebe084f6318ce4158352c6b6de836cf7de Mon Sep 17 00:00:00 2001 From: George Date: Wed, 3 Jul 2024 12:11:43 +0300 Subject: [PATCH 02/18] Disable screenshot diff check for e2e playwright tests Signed-off-by: George --- web-console/playwright-e2e.config.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web-console/playwright-e2e.config.ts b/web-console/playwright-e2e.config.ts index 1b1fc1a55a..05c32e5ba0 100644 --- a/web-console/playwright-e2e.config.ts +++ b/web-console/playwright-e2e.config.ts @@ -36,6 +36,11 @@ export default defineConfig({ trace: 'on-first-retry', testIdAttribute: 'data-testid', }, + expect: { + toHaveScreenshot: { + maxDiffPixelRatio: 1 + } + }, /* Configure projects for major browsers */ projects: [ From 55c7200981720e6970e5deaa2a081c22e6d6873a Mon Sep 17 00:00:00 2001 From: George Date: Fri, 5 Jul 2024 13:49:44 +0300 Subject: [PATCH 03/18] Temporarily disable e2e tests in CI Signed-off-by: George --- .arg | 2 +- Earthfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.arg b/.arg index 8534f29ba4..9cb9876cde 100644 --- a/.arg +++ b/.arg @@ -1,2 +1,2 @@ # File used by earthly -PLAYWRIGHT_SNAPSHOTS_COMMIT=7e91647f7b85cfc472c61271d6115c566851b76c +PLAYWRIGHT_SNAPSHOTS_COMMIT=983dd831d58229a12a0150dbb65c0ff04394671c diff --git a/Earthfile b/Earthfile index a9a1f03cdf..ba270cf84d 100644 --- a/Earthfile +++ b/Earthfile @@ -590,7 +590,7 @@ all-tests: BUILD +test-sql BUILD +integration-tests BUILD +ui-playwright-tests-ct - BUILD +ui-playwright-tests-e2e + # BUILD +ui-playwright-tests-e2e BUILD +test-docker-compose # BUILD +test-docker-compose-stable BUILD +test-debezium-mysql From 092bc3d79f8aab15c5a89b030c69797df46f78b9 Mon Sep 17 00:00:00 2001 From: "Matei Budiu (Aehmttw)" Date: Mon, 8 Jul 2024 14:22:59 -0700 Subject: [PATCH 04/18] Add script to run flink on CI (#1985) Add manually-triggered Flink benchmark task to Earthly. --------- Signed-off-by: Matei --- Earthfile | 21 +++++++++++++++++++++ benchmark/flink/setup-flink.sh | 6 ++++++ 2 files changed, 27 insertions(+) create mode 100755 benchmark/flink/setup-flink.sh diff --git a/Earthfile b/Earthfile index ba270cf84d..e12967f5c1 100644 --- a/Earthfile +++ b/Earthfile @@ -580,6 +580,27 @@ benchmark: SAVE ARTIFACT crates/dbsp/galen_results.csv AS LOCAL . #SAVE ARTIFACT crates/dbsp/ldbc_results.csv AS LOCAL . +flink-benchmark: + FROM +rust-sources + + # Install docker compose - earthly can do this automatically, but it installs an older version + ENV DOCKER_CONFIG=${DOCKER_CONFIG:-$HOME/.docker} + RUN mkdir -p $DOCKER_CONFIG/cli-plugins + RUN curl -SL https://github.com/docker/compose/releases/download/v2.24.0-birthday.10/docker-compose-linux-x86_64 -o $DOCKER_CONFIG/cli-plugins/docker-compose + RUN chmod +x $DOCKER_CONFIG/cli-plugins/docker-compose + RUN mkdir -p benchmark/flink + COPY benchmark/flink/* benchmark/flink + COPY benchmark/run-nexmark.sh benchmark + RUN apt-get install maven + RUN benchmark/flink/setup-flink.sh + + RUN echo "when,runner,mode,language,name,num_cores,num_events,elapsed" >> flink_results.csv + WITH DOCKER + RUN docker compose -f benchmark/flink/docker-compose.yml -p nexmark up --build --force-recreate --renew-anon-volumes -d && \ + docker exec -i nexmark-jobmanager-1 run.sh 2>&1 | ./benchmark/run-nexmark.sh --runner flink --parse >> flink_results.csv + END + SAVE ARTIFACT flink_results.csv AS LOCAL . + all-tests: BUILD +formatting-check BUILD +machete diff --git a/benchmark/flink/setup-flink.sh b/benchmark/flink/setup-flink.sh new file mode 100755 index 0000000000..3a053de27e --- /dev/null +++ b/benchmark/flink/setup-flink.sh @@ -0,0 +1,6 @@ +#!/bin/sh -x +cd "$(dirname "$0")" +rm -rf nexmark nexmark-flink +git clone https://github.com/blp/nexmark.git +(cd nexmark/nexmark-flink && ./build.sh) +tar xzf nexmark/nexmark-flink/nexmark-flink.tgz \ No newline at end of file From 271ccfdc04ab9acb46cbd8886f67a7d208da3b13 Mon Sep 17 00:00:00 2001 From: Mihai Budiu Date: Mon, 8 Jul 2024 16:10:26 -0700 Subject: [PATCH 05/18] [SQL] a few small optimizations: combine join/filter/map and map/mapindex (#1986) Signed-off-by: Mihai Budiu --- .../operator/DBSPIndexedTopKOperator.java | 6 +- ...apOperator.java => DBSPJoinFilterMap.java} | 33 +++++++--- ...PartitionedRadixTreeAggregateOperator.java | 65 ------------------- .../compiler/backend/ToDotVisitor.java | 6 ++ .../backend/rust/LowerCircuitVisitor.java | 62 ++++++++++++++++++ .../backend/rust/ToRustInnerVisitor.java | 52 +++++++++------ .../calciteCompiler/CalciteOptimizer.java | 20 +++--- .../visitors/outer/CircuitCloneVisitor.java | 7 +- .../visitors/outer/CircuitOptimizer.java | 1 + .../visitors/outer/CircuitRewriter.java | 12 ++-- .../visitors/outer/CircuitVisitor.java | 12 +--- .../compiler/visitors/outer/FilterJoin.java | 30 ++------- .../outer/OptimizeProjectionVisitor.java | 36 +++++++++- .../ir/expression/DBSPClosureExpression.java | 7 +- .../ir/expression/DBSPExpression.java | 11 ++++ .../src/main/java/org/dbsp/util/Logger.java | 13 ++-- .../sqlCompiler/compiler/sql/OtherTests.java | 21 ------ .../sql/suites/nexmark/NexmarkTest.java | 25 ++++--- 18 files changed, 221 insertions(+), 198 deletions(-) rename sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/{DBSPJoinFlatmapOperator.java => DBSPJoinFilterMap.java} (57%) delete mode 100644 sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPPartitionedRadixTreeAggregateOperator.java diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPIndexedTopKOperator.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPIndexedTopKOperator.java index f3382ff965..31f493ce90 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPIndexedTopKOperator.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPIndexedTopKOperator.java @@ -33,11 +33,9 @@ public enum TopKNumbering { public final TopKNumbering numbering; /** Limit K used by TopK. Expected to be a constant */ public final DBSPExpression limit; - /** - * Optional closure which produces the output tuple. The signature is + /** Optional closure which produces the output tuple. The signature is * (i64, sorted_tuple) -> output_tuple. i64 is the rank of the current row. - * If this closure is missing it is assumed to produce just the sorted_tuple. - */ + * If this closure is missing it is assumed to produce just the sorted_tuple. */ @Nullable public final DBSPClosureExpression outputProducer; diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPJoinFlatmapOperator.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPJoinFilterMap.java similarity index 57% rename from sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPJoinFlatmapOperator.java rename to sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPJoinFilterMap.java index 5bd29146fb..6440006ec6 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPJoinFlatmapOperator.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPJoinFilterMap.java @@ -11,31 +11,44 @@ import java.util.List; import java.util.Objects; -/** We use join_flatmap to implement a join followed by a filter. - * The function returns None for dropping a value, and Some(result) - * for keeping a result. */ -public final class DBSPJoinFlatmapOperator extends DBSPBinaryOperator { - public DBSPJoinFlatmapOperator( +/** This class represents a join followed by a filter followed by a map. + * The operator is eventually lowered to a join_flatmap, + * where the synthesized function + * returns None when filter(function) is false, and Some(map(function)) + * otherwise. */ +public final class DBSPJoinFilterMap extends DBSPBinaryOperator { + // If the following is null, the function represents the combined function/filter + // and the function returns Option. + @Nullable + public final DBSPExpression filter; + @Nullable + public final DBSPExpression map; + + public DBSPJoinFilterMap( CalciteObject node, DBSPTypeZSet outputType, - DBSPExpression function, boolean isMultiset, + DBSPExpression function, @Nullable DBSPExpression filter, @Nullable DBSPExpression map, + boolean isMultiset, DBSPOperator left, DBSPOperator right) { super(node, "join_flatmap", function, outputType, isMultiset, left, right); + this.filter = filter; + this.map = map; } @Override public DBSPOperator withFunction(@Nullable DBSPExpression expression, DBSPType outputType) { - return new DBSPJoinFlatmapOperator( + return new DBSPJoinFilterMap( this.getNode(), outputType.to(DBSPTypeZSet.class), - Objects.requireNonNull(expression), + Objects.requireNonNull(expression), this.filter, this.map, this.isMultiset, this.left(), this.right()); } @Override public DBSPOperator withInputs(List newInputs, boolean force) { if (force || this.inputsDiffer(newInputs)) - return new DBSPJoinFlatmapOperator( + return new DBSPJoinFilterMap( this.getNode(), this.getOutputZSetType(), - this.getFunction(), this.isMultiset, newInputs.get(0), newInputs.get(1)); + this.getFunction(), this.filter, this.map, + this.isMultiset, newInputs.get(0), newInputs.get(1)); return this; } diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPPartitionedRadixTreeAggregateOperator.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPPartitionedRadixTreeAggregateOperator.java deleted file mode 100644 index fe64e63d22..0000000000 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/circuit/operator/DBSPPartitionedRadixTreeAggregateOperator.java +++ /dev/null @@ -1,65 +0,0 @@ -package org.dbsp.sqlCompiler.circuit.operator; - -import org.dbsp.sqlCompiler.compiler.frontend.calciteObject.CalciteObject; -import org.dbsp.sqlCompiler.compiler.visitors.VisitDecision; -import org.dbsp.sqlCompiler.compiler.visitors.inner.EquivalenceContext; -import org.dbsp.sqlCompiler.compiler.visitors.outer.CircuitVisitor; -import org.dbsp.sqlCompiler.ir.DBSPAggregate; -import org.dbsp.sqlCompiler.ir.NonCoreIR; -import org.dbsp.sqlCompiler.ir.expression.DBSPExpression; -import org.dbsp.sqlCompiler.ir.type.DBSPType; - -import javax.annotation.Nullable; -import java.util.List; - -@NonCoreIR -public final class DBSPPartitionedRadixTreeAggregateOperator extends DBSPOperator { - @Nullable - public final DBSPAggregate aggregate; - - public DBSPPartitionedRadixTreeAggregateOperator( - CalciteObject node, @Nullable DBSPExpression function, @Nullable DBSPAggregate aggregate, - DBSPOperator input, DBSPOperator inputIntegral, DBSPOperator outputIntegral) { - super(node, "PartitionedRadixTreeAggregate", function, outputIntegral.getType(), false); - this.addInput(input); - this.addInput(inputIntegral); - this.addInput(outputIntegral); - this.aggregate = aggregate; - } - - @Override - public DBSPOperator withFunction(@Nullable DBSPExpression expression, DBSPType outputType) { - return new DBSPPartitionedRadixTreeAggregateOperator(this.getNode(), expression, this.aggregate, - this.inputs.get(0), this.inputs.get(1), this.inputs.get(2)); - } - - @Override - public DBSPOperator withInputs(List newInputs, boolean force) { - assert newInputs.size() == 3: "Expected 3 inputs"; - if (force || this.inputsDiffer(newInputs)) - return new DBSPPartitionedRadixTreeAggregateOperator( - this.getNode(), this.getFunction(), this.aggregate, - newInputs.get(0), newInputs.get(1), newInputs.get(2)); - return this; - } - - @Override - public boolean equivalent(DBSPOperator other) { - if (!super.equivalent(other)) - return false; - DBSPPartitionedRadixTreeAggregateOperator otherOperator = - other.as(DBSPPartitionedRadixTreeAggregateOperator.class); - if (otherOperator == null) - return false; - return EquivalenceContext.equiv(this.aggregate, otherOperator.aggregate); - } - - @Override - public void accept(CircuitVisitor visitor) { - visitor.push(this); - VisitDecision decision = visitor.preorder(this); - if (!decision.stop()) - visitor.postorder(this); - visitor.pop(this); - } -} diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/ToDotVisitor.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/ToDotVisitor.java index e3ebeb6830..7042520fa9 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/ToDotVisitor.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/ToDotVisitor.java @@ -30,6 +30,7 @@ import org.dbsp.sqlCompiler.circuit.operator.DBSPDelayOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPDelayOutputOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPFlatMapOperator; +import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinFilterMap; import org.dbsp.sqlCompiler.circuit.operator.DBSPOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPPartitionedRollingAggregateWithWaterlineOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPSourceBaseOperator; @@ -178,6 +179,11 @@ String getFunction(DBSPOperator node) { expression = LowerCircuitVisitor.rewriteFlatmap(expression.to(DBSPFlatmap.class)); } } + if (node.is(DBSPJoinFilterMap.class)) { + expression = LowerCircuitVisitor.lowerJoinFilterMapFunctions( + this.errorReporter, + node.to(DBSPJoinFilterMap.class)); + } String result = ToRustInnerVisitor.toRustString( this.errorReporter, expression, CompilerOptions.getDefault(), true); result = result.replace("\n", "\\l"); diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/LowerCircuitVisitor.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/LowerCircuitVisitor.java index 71e38d0a6e..de91d7454a 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/LowerCircuitVisitor.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/LowerCircuitVisitor.java @@ -2,6 +2,7 @@ import org.dbsp.sqlCompiler.circuit.operator.DBSPAggregateOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPFlatMapOperator; +import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinFilterMap; import org.dbsp.sqlCompiler.circuit.operator.DBSPOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPPartitionedRollingAggregateOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPPartitionedRollingAggregateWithWaterlineOperator; @@ -204,6 +205,67 @@ public void postorder(DBSPAggregateOperator node) { this.map(node, result); } + public static DBSPClosureExpression lowerJoinFilterMapFunctions( + IErrorReporter errorReporter, DBSPJoinFilterMap node) { + assert node.filter != null; + if (node.map == null) { + // Generate code of the form + // let tmp = join(...); + // if (filter(tmp)) { + // Some(tmp) + // } else { + // None + // } + DBSPClosureExpression expression = node.getFunction().to(DBSPClosureExpression.class); + DBSPClosureExpression filter = node.filter.to(DBSPClosureExpression.class); + DBSPLetStatement let = new DBSPLetStatement("tmp", expression.body); + DBSPExpression cond = filter.call(let.getVarReference().borrow()); + DBSPExpression tmp = let.getVarReference(); + DBSPIfExpression ifexp = new DBSPIfExpression( + node.getNode(), + cond, + tmp.some(), + tmp.getType().setMayBeNull(true).none()); + DBSPBlockExpression block = new DBSPBlockExpression(Linq.list(let), ifexp); + return block.closure(expression.parameters); + } else { + // Generate code of the form + // if (filter(join(...)) { + // Some(map(join(...))) + // } else { + // None + // } + DBSPClosureExpression expression = node.getFunction().to(DBSPClosureExpression.class); + DBSPClosureExpression filter = node.filter.to(DBSPClosureExpression.class); + DBSPExpression cond = filter + .call(expression.body.borrow()) + .reduce(errorReporter); + DBSPExpression map = node.map.to(DBSPClosureExpression.class) + .call(expression.body.borrow()) + .reduce(errorReporter); + DBSPIfExpression ifexp = new DBSPIfExpression( + node.getNode(), + cond, + map.some(), + map.getType().setMayBeNull(true).none()); + return ifexp.closure(expression.parameters); + } + } + + @Override + public void postorder(DBSPJoinFilterMap node) { + if (node.filter == null) { + // Already lowered + super.postorder(node); + return; + } + DBSPExpression newFunction = lowerJoinFilterMapFunctions(this.errorReporter, node); + DBSPOperator result = new DBSPJoinFilterMap(node.getNode(), node.getOutputZSetType(), + newFunction, null, null, node.isMultiset, + this.mapped(node.left()), this.mapped(node.right())); + this.map(node, result); + } + @Override public void postorder(DBSPPartitionedRollingAggregateOperator node) { if (node.aggregate == null) { diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/ToRustInnerVisitor.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/ToRustInnerVisitor.java index dbd67533d5..a52e2b8691 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/ToRustInnerVisitor.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/ToRustInnerVisitor.java @@ -1274,34 +1274,50 @@ public VisitDecision preorder(DBSPTypeFunction type) { @Override public VisitDecision preorder(DBSPTypeBaseType type) { - type.wrapOption(this.builder, type.getRustString()); + if (this.compact) { + this.builder.append(type.getRustString()); + if (type.mayBeNull) + this.builder.append("?"); + } else { + type.wrapOption(this.builder, type.getRustString()); + } return VisitDecision.STOP; } + void optionPrefix(DBSPType type) { + if (type.mayBeNull && !this.compact) + this.builder.append("Option<"); + } + + void optionSuffix(DBSPType type) { + if (type.mayBeNull) { + if (this.compact) + this.builder.append("?"); + else + this.builder.append(">"); + } + } + @Override public VisitDecision preorder(DBSPTypeRawTuple type) { - if (type.mayBeNull) - this.builder.append("Option<"); + this.optionPrefix(type); this.builder.append("("); for (DBSPType fType: type.tupFields) { fType.accept(this); this.builder.append(", "); } this.builder.append(")"); - if (type.mayBeNull) - this.builder.append(">"); + this.optionSuffix(type); return VisitDecision.STOP; } @Override public VisitDecision preorder(DBSPTypeRef type) { - if (type.mayBeNull) - this.builder.append("Option<"); + this.optionPrefix(type); this.builder.append("&") .append(type.mutable ? "mut " : ""); type.type.accept(this); - if (type.mayBeNull) - this.builder.append(">"); + this.optionSuffix(type); return VisitDecision.STOP; } @@ -1348,18 +1364,15 @@ public VisitDecision preorder(DBSPStructItem item) { @Override public VisitDecision preorder(DBSPTypeStruct type) { // A *reference* to a struct type is just the type name. - if (type.mayBeNull) - this.builder.append("Option<"); + this.optionPrefix(type); this.builder.append(type.sanitizedName); - if (type.mayBeNull) - this.builder.append(">"); + this.optionSuffix(type); return VisitDecision.STOP; } @Override public VisitDecision preorder(DBSPTypeTuple type) { - if (type.mayBeNull) - this.builder.append("Option<"); + this.optionPrefix(type); this.builder.append(type.getName()); if (type.size() > 0) { this.builder.append("<"); @@ -1372,15 +1385,13 @@ public VisitDecision preorder(DBSPTypeTuple type) { } this.builder.append(">"); } - if (type.mayBeNull) - this.builder.append(">"); + this.optionSuffix(type); return VisitDecision.STOP; } @Override public VisitDecision preorder(DBSPTypeUser type) { - if (type.mayBeNull) - this.builder.append("Option<"); + this.optionPrefix(type); this.builder.append(type.name); if (type.typeArgs.length > 0) { this.builder.append("<"); @@ -1393,8 +1404,7 @@ public VisitDecision preorder(DBSPTypeUser type) { } this.builder.append(">"); } - if (type.mayBeNull) - this.builder.append(">"); + this.optionSuffix(type); return VisitDecision.STOP; } diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/frontend/calciteCompiler/CalciteOptimizer.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/frontend/calciteCompiler/CalciteOptimizer.java index b790619d3c..9d2b137f16 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/frontend/calciteCompiler/CalciteOptimizer.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/frontend/calciteCompiler/CalciteOptimizer.java @@ -113,8 +113,7 @@ static class OuterJoinFinder extends RelVisitor { @Override public void visit( RelNode node, int ordinal, @org.checkerframework.checker.nullness.qual.Nullable RelNode parent) { - if (node instanceof Join) { - Join join = (Join)node; + if (node instanceof Join join) { ++joinCount; if (join.getJoinType().isOuterJoin()) ++outerJoinCount; @@ -189,21 +188,24 @@ HepProgram getProgram(RelNode node) { } }); + SimpleOptimizerStep merge = new SimpleOptimizerStep( + "Merge identical operations", + CoreRules.PROJECT_MERGE, + CoreRules.MINUS_MERGE, + CoreRules.UNION_MERGE, + CoreRules.AGGREGATE_MERGE, + CoreRules.INTERSECT_MERGE); + // this.addStep(merge); -- messes up the shape of uncollect this.addStep(new SimpleOptimizerStep( "Move projections", CoreRules.PROJECT_CORRELATE_TRANSPOSE, + CoreRules.PROJECT_WINDOW_TRANSPOSE, CoreRules.PROJECT_SET_OP_TRANSPOSE, CoreRules.FILTER_PROJECT_TRANSPOSE // Rule is unsound // CoreRules.PROJECT_JOIN_TRANSPOSE )); - this.addStep(new SimpleOptimizerStep( - "Merge identical operations", - CoreRules.PROJECT_MERGE, - CoreRules.MINUS_MERGE, - CoreRules.UNION_MERGE, - CoreRules.AGGREGATE_MERGE, - CoreRules.INTERSECT_MERGE)); + this.addStep(merge); this.addStep(new SimpleOptimizerStep("Remove dead code", CoreRules.AGGREGATE_REMOVE, CoreRules.UNION_REMOVE, diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitCloneVisitor.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitCloneVisitor.java index 75776acef4..c8bd26b7d7 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitCloneVisitor.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitCloneVisitor.java @@ -202,11 +202,6 @@ public void postorder(DBSPDelayOperator operator) { @Override public void postorder(DBSPUnaryOperator operator) { this.replace(operator); } - @Override - public void postorder(DBSPPartitionedRadixTreeAggregateOperator operator) { - this.replace(operator); - } - @Override public void postorder(DBSPStreamAggregateOperator operator) { this.replace(operator); @@ -316,7 +311,7 @@ public void postorder(DBSPJoinOperator operator) { } @Override - public void postorder(DBSPJoinFlatmapOperator operator) { + public void postorder(DBSPJoinFilterMap operator) { this.replace(operator); } diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitOptimizer.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitOptimizer.java index 4818c95ee0..3afe8fe125 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitOptimizer.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitOptimizer.java @@ -82,6 +82,7 @@ CircuitTransform getOptimizer() { passes.add(new ExpandHop(reporter)); passes.add(new RemoveDeindexOperators(reporter)); passes.add(new RemoveViewOperators(reporter)); + passes.add(new OptimizeProjections(reporter)); passes.add(new EliminateFunctions(reporter).circuitRewriter()); passes.add(new ExpandWriteLog(reporter).circuitRewriter()); passes.add(new Simplify(reporter).circuitRewriter()); diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitRewriter.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitRewriter.java index 8c86d64713..7e975bb440 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitRewriter.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitRewriter.java @@ -30,7 +30,7 @@ import org.dbsp.sqlCompiler.circuit.DBSPDeclaration; import org.dbsp.sqlCompiler.circuit.operator.DBSPFlatMapOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPIntegrateTraceRetainKeysOperator; -import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinFlatmapOperator; +import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinFilterMap; import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPLagOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPMapIndexOperator; @@ -318,16 +318,20 @@ public void postorder(DBSPStreamJoinOperator operator) { } @Override - public void postorder(DBSPJoinFlatmapOperator operator) { + public void postorder(DBSPJoinFilterMap operator) { DBSPType outputType = this.transform(operator.outputType); DBSPExpression function = this.transform(operator.getFunction()); + DBSPExpression filter = this.transformN(operator.filter); + DBSPExpression map = this.transformN(operator.map); List sources = Linq.map(operator.inputs, this::mapped); DBSPOperator result = operator; if (!outputType.sameType(operator.outputType) || function != operator.function + || filter != operator.filter + || map != operator.map || Linq.different(sources, operator.inputs)) { - result = new DBSPJoinFlatmapOperator(operator.getNode(), outputType.to(DBSPTypeZSet.class), - function, operator.isMultiset, sources.get(0), sources.get(1)); + result = new DBSPJoinFilterMap(operator.getNode(), outputType.to(DBSPTypeZSet.class), + function, filter, map, operator.isMultiset, sources.get(0), sources.get(1)); } this.map(operator, result); } diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitVisitor.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitVisitor.java index 439b746f5d..0937389ac1 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitVisitor.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/CircuitVisitor.java @@ -166,10 +166,6 @@ public VisitDecision preorder(DBSPStreamAggregateOperator node) { return this.preorder((DBSPAggregateOperatorBase) node); } - public VisitDecision preorder(DBSPPartitionedRadixTreeAggregateOperator node) { - return this.preorder(node.to(DBSPOperator.class)); - } - public VisitDecision preorder(DBSPAggregateOperator node) { return this.preorder((DBSPAggregateOperatorBase) node); } @@ -278,7 +274,7 @@ public VisitDecision preorder(DBSPJoinOperator node) { return this.preorder(node.to(DBSPBinaryOperator.class)); } - public VisitDecision preorder(DBSPJoinFlatmapOperator node) { + public VisitDecision preorder(DBSPJoinFilterMap node) { return this.preorder(node.to(DBSPBinaryOperator.class)); } @@ -356,7 +352,7 @@ public void postorder(DBSPJoinOperator node) { this.postorder(node.to(DBSPBinaryOperator.class)); } - public void postorder(DBSPJoinFlatmapOperator node) { + public void postorder(DBSPJoinFilterMap node) { this.postorder(node.to(DBSPBinaryOperator.class)); } @@ -368,10 +364,6 @@ public void postorder(DBSPStreamAggregateOperator node) { this.postorder((DBSPAggregateOperatorBase) node); } - public void postorder(DBSPPartitionedRadixTreeAggregateOperator node) { - this.postorder(node.to(DBSPOperator.class)); - } - public void postorder(DBSPAggregateOperator node) { this.postorder((DBSPAggregateOperatorBase) node); } diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/FilterJoin.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/FilterJoin.java index bbd3f01e7d..12adeaf652 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/FilterJoin.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/FilterJoin.java @@ -1,21 +1,12 @@ package org.dbsp.sqlCompiler.compiler.visitors.outer; import org.dbsp.sqlCompiler.circuit.operator.DBSPFilterOperator; -import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinFlatmapOperator; +import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinFilterMap; import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPOperator; import org.dbsp.sqlCompiler.compiler.IErrorReporter; -import org.dbsp.sqlCompiler.compiler.visitors.inner.BetaReduction; -import org.dbsp.sqlCompiler.ir.expression.DBSPBlockExpression; -import org.dbsp.sqlCompiler.ir.expression.DBSPClosureExpression; -import org.dbsp.sqlCompiler.ir.expression.DBSPExpression; -import org.dbsp.sqlCompiler.ir.expression.DBSPIfExpression; -import org.dbsp.sqlCompiler.ir.statement.DBSPLetStatement; -import org.dbsp.util.Linq; -import java.util.List; - -/** Combine a Join followed by a filter into a JoinFlatmap. */ +/** Combine a Join followed by a filter into a JoinFilterMap. */ public class FilterJoin extends CircuitCloneVisitor { public FilterJoin(IErrorReporter reporter) { super(reporter, false); @@ -25,21 +16,10 @@ public FilterJoin(IErrorReporter reporter) { public void postorder(DBSPFilterOperator operator) { DBSPOperator source = this.mapped(operator.input()); if (source.is(DBSPJoinOperator.class)) { - DBSPClosureExpression expression = source.getFunction().to(DBSPClosureExpression.class); - DBSPClosureExpression filter = operator.getFunction().to(DBSPClosureExpression.class); - DBSPLetStatement let = new DBSPLetStatement("tmp", expression.body); - DBSPExpression cond = filter.call(let.getVarReference().borrow()); - DBSPIfExpression ifexp = new DBSPIfExpression( - source.getNode(), - cond, - let.getVarReference().some(), - let.getVarReference().getType().setMayBeNull(true).none()); - DBSPBlockExpression block = new DBSPBlockExpression(Linq.list(let), ifexp); - DBSPClosureExpression newFunction = block.closure(expression.parameters); - newFunction = new BetaReduction(this.errorReporter).apply(newFunction).to(DBSPClosureExpression.class); DBSPOperator result = - new DBSPJoinFlatmapOperator(source.getNode(), source.getOutputZSetType(), - newFunction, source.isMultiset, source.inputs.get(0), source.inputs.get(1)); + new DBSPJoinFilterMap(source.getNode(), source.getOutputZSetType(), + source.getFunction(), operator.getFunction(), null, + source.isMultiset, source.inputs.get(0), source.inputs.get(1)); this.map(operator, result); return; } diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/OptimizeProjectionVisitor.java b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/OptimizeProjectionVisitor.java index d95b9a2d9d..4cf507a441 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/OptimizeProjectionVisitor.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/visitors/outer/OptimizeProjectionVisitor.java @@ -5,13 +5,14 @@ import org.dbsp.sqlCompiler.circuit.operator.DBSPDifferentiateOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPFlatMapOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPIntegrateOperator; +import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinFilterMap; import org.dbsp.sqlCompiler.circuit.operator.DBSPJoinOperator; +import org.dbsp.sqlCompiler.circuit.operator.DBSPMapIndexOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPMapOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPNegateOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPNoopOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPOperator; import org.dbsp.sqlCompiler.circuit.operator.DBSPStreamJoinOperator; -import org.dbsp.sqlCompiler.circuit.operator.DBSPSumOperator; import org.dbsp.sqlCompiler.compiler.IErrorReporter; import org.dbsp.sqlCompiler.compiler.visitors.inner.Projection; import org.dbsp.sqlCompiler.ir.expression.DBSPClosureExpression; @@ -32,7 +33,7 @@ * structure. The function is analyzed using the 'Projection' inner visitor. * - Merge Projection operations into the previous operation if possible. * Done for joins, constants, flatmaps, and some maps. - * - Swap projection with operations such as Distinct, Integral, Differential, Sum, etc. + * - Swap projection with operations such as Distinct, Integral, Differential, etc. */ public class OptimizeProjectionVisitor extends CircuitCloneVisitor { /** If this function returns 'true' the operator can be optimized. */ @@ -43,10 +44,39 @@ public OptimizeProjectionVisitor(IErrorReporter reporter, Function int getLoggingLevel(Class clazz) { return this.loggingLevel.getOrDefault(clazz.getSimpleName(), 0); } - public void setLoggingLevel(Class clazz, int level) { - this.setLoggingLevel(clazz.getSimpleName(), level); + public int setLoggingLevel(Class clazz, int level) { + return this.setLoggingLevel(clazz.getSimpleName(), level); } /** diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/OtherTests.java b/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/OtherTests.java index a62079c715..3c601c301c 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/OtherTests.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/OtherTests.java @@ -569,29 +569,8 @@ public void testProjectionSimplify() { String sql = EndToEndTests.E2E_TABLE + "; " + "CREATE VIEW V AS SELECT T1.COL3 FROM T AS T1 JOIN T AS T2 ON T1.COL2 = T2.COL6"; DBSPCompiler compiler = this.testCompiler(); - // Without optimizations there is a map operator. - compiler.options.languageOptions.optimizationLevel = 1; compiler.compileStatements(sql); DBSPCircuit circuit = getCircuit(compiler); - CircuitVisitor findMap = new CircuitVisitor(compiler) { - int mapCount = 0; - - @Override - public VisitDecision preorder(DBSPMapOperator node) { - this.mapCount++; - return super.preorder(node); - } - - @Override - public void endVisit() { - Assert.assertTrue(this.mapCount > 0); - } - }; - findMap.apply(circuit); - - compiler = this.testCompiler(); - compiler.compileStatements(sql); - circuit = getCircuit(compiler); CircuitVisitor noMap = new CircuitVisitor(compiler) { @Override public VisitDecision preorder(DBSPMapOperator node) { diff --git a/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/suites/nexmark/NexmarkTest.java b/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/suites/nexmark/NexmarkTest.java index 6a422d4188..a00e5a918a 100644 --- a/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/suites/nexmark/NexmarkTest.java +++ b/sql-to-dbsp-compiler/SQL-compiler/src/test/java/org/dbsp/sqlCompiler/compiler/sql/suites/nexmark/NexmarkTest.java @@ -27,6 +27,7 @@ import org.dbsp.sqlCompiler.compiler.CompilerOptions; import org.dbsp.sqlCompiler.compiler.DBSPCompiler; import org.dbsp.sqlCompiler.compiler.sql.StreamingTest; +import org.dbsp.util.Logger; import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; @@ -556,6 +557,7 @@ public DBSPCompiler testCompiler(boolean optimize) { CompilerOptions options = new CompilerOptions(); options.languageOptions.lexicalRules = Lex.ORACLE; options.languageOptions.throwOnError = true; + options.languageOptions.incrementalize = true; options.languageOptions.generateInputForEveryTable = false; options.ioOptions.emitHandles = true; return new DBSPCompiler(options); @@ -566,7 +568,14 @@ void createTest(int query, String... scriptsAndTables) { DBSPCompiler compiler = this.testCompiler(); this.prepareInputs(compiler); compiler.compileStatements(queries[query]); + final boolean debug = false; + Class module = DBSPCompiler.class; + int previous; + if (debug) + previous = Logger.INSTANCE.setLoggingLevel(module, 4); CompilerCircuitStream ccs = new CompilerCircuitStream(compiler); + if (debug) + Logger.INSTANCE.setLoggingLevel(module, previous); for (int i = 0; i < scriptsAndTables.length; i += 2) ccs.step(scriptsAndTables[i], scriptsAndTables[i + 1]); this.addRustTestCase("q" + query, ccs); @@ -710,7 +719,7 @@ public void q4Test() { } @Test @Ignore("The results are wrong, must investigate") - public void testQ7() { + public void q7test() { this.createTest(7, // The rust code has transposed columns 'price' and 'bidder' in the output """ @@ -729,7 +738,7 @@ public void testQ7() { } @Test - public void testQ8() { + public void q8test() { // Persons 2 and 3 were both added during the 10-20 interval and created auctions in // that same interval. Person 1 was added in the previous interval (0-10) though their // auction is in the correct interval. Person 4 was added in the interval, but their auction is @@ -785,14 +794,14 @@ public void testQ8() { } @Test - public void testQ9() { + public void q9test() { this.createTest(9, "", """ id | item | description | initialBid | reserve | date_time | expires | seller | category | extra | auction | bidder | price | bid_datetime | bid_extra | weight -----------------------------------------------------------------------------------------------------------------------------------------------------------------"""); } @Test - public void testQ10() { + public void q10test() { this.createTest(10, "", """ auction | bidder | price | date_time | extra | date | time | weight @@ -800,7 +809,7 @@ public void testQ10() { } @Test - public void testQ17() { + public void q17test() { this.createTest(17, "", """ auction | date | total_bids | rank1_bids | rank2_bids | rank3_bids | min_price | max_price | avg_price | sum_price | weight @@ -808,7 +817,7 @@ public void testQ17() { } @Test - public void testQ18() { + public void q18test() { this.createTest(18, "", """ auction | bidder | price | channel | url | date_time | extra | weight @@ -816,7 +825,7 @@ public void testQ18() { } @Test - public void testQ19() { + public void q19test() { this.createTest(19, "", """ auction | bidder | price | channel | url | date_time | extra | row_number | weight @@ -824,7 +833,7 @@ public void testQ19() { } @Test - public void testQ20() { + public void q20test() { this.createTest(19, "", """ auction | bidder | price | channel | url | date_time | extra | itemName | description | initialBid | reserve | ADateTime | expires | seller | category | Aextra | weight From 3c2012ade3e1aecb8d842f50fad23efeaa9eaa32 Mon Sep 17 00:00:00 2001 From: George Date: Mon, 8 Jul 2024 09:11:37 +0000 Subject: [PATCH 06/18] Disable navigation from materialized to non-materialized view Signed-off-by: George --- .../(app)/streaming/inspection/page.tsx | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/inspection/page.tsx b/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/inspection/page.tsx index 793ed632dd..e76878ae50 100644 --- a/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/inspection/page.tsx +++ b/web-console/src/app/(spa)/(root)/(authenticated)/(app)/streaming/inspection/page.tsx @@ -12,7 +12,7 @@ import { useSearchParams } from 'next/navigation' import { useEffect } from 'react' import { nonNull } from 'src/lib/functions/common/function' -import { Alert, AlertTitle, Autocomplete, Box, FormControl, Link, MenuItem, TextField } from '@mui/material' +import { Alert, AlertTitle, Autocomplete, Box, FormControl, Link, MenuItem, TextField, Tooltip } from '@mui/material' import Grid from '@mui/material/Grid' import { useQuery } from '@tanstack/react-query' import { useHashPart } from 'src/lib/compositions/useHashPart' @@ -26,7 +26,7 @@ const TablesBreadcrumb = (props: { }) => { const [tab] = useHashPart() const options = props.tables - .map(relation => ({ type: 'Tables', name: getCaseIndependentName(relation), relation })) + .map(relation => ({ type: 'Tables' as 'Tables' | 'Views', name: getCaseIndependentName(relation), relation })) .concat(props.views.map(relation => ({ type: 'Views', name: getCaseIndependentName(relation), relation }))) return ( @@ -47,19 +47,36 @@ const TablesBreadcrumb = (props: { type: props.relationType === 'table' ? 'Tables' : 'Views', relation: undefined! }} - renderOption={(_props, item) => ( - - {item.name} - - )} + renderOption={(_props, item) => { + const baseUrl = `?pipeline_name=${props.pipeline.descriptor.name}&relation=${item.name}` + const href = + item.type === 'Tables' + ? baseUrl + (item.relation.materialized ? '#' + tab : '#insert') + : item.relation.materialized + ? baseUrl + '#inspect' + : undefined + const disabled = !href?.length + return ( + + + {item.name} + + + ) + }} /> From cc1d62f66c19af965d615ae888a43522a7316758 Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Tue, 9 Jul 2024 09:50:43 +0200 Subject: [PATCH 07/18] release: bump project version to 0.20.0 Signed-off-by: Gerd Zellweger --- CHANGELOG.md | 2 ++ Cargo.lock | 2 +- crates/pipeline_manager/Cargo.toml | 2 +- openapi.json | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abb5bd9c93..a1bcdcb127 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.20.0] - 2024-07-09 + - [SQL] Added `MATERIALIZED` views ([#1959](https://github.com/feldera/feldera/pull/1959)) - [WebConsole] Drop Data Services functionality (#1945) diff --git a/Cargo.lock b/Cargo.lock index db8a2f4f0e..693af38d01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6918,7 +6918,7 @@ checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" [[package]] name = "pipeline-manager" -version = "0.19.0" +version = "0.20.0" dependencies = [ "actix-cors", "actix-files", diff --git a/crates/pipeline_manager/Cargo.toml b/crates/pipeline_manager/Cargo.toml index 72d1dcd68c..27b9af6ea8 100644 --- a/crates/pipeline_manager/Cargo.toml +++ b/crates/pipeline_manager/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pipeline-manager" -version = "0.19.0" +version = "0.20.0" edition = "2021" license = "MIT OR Apache-2.0" description = "Data pipeline manager for the Feldera continuous analytics platform." diff --git a/openapi.json b/openapi.json index 765b482345..7e8b2794bd 100644 --- a/openapi.json +++ b/openapi.json @@ -6,7 +6,7 @@ "license": { "name": "MIT OR Apache-2.0" }, - "version": "0.19.0" + "version": "0.20.0" }, "paths": { "/config/authentication": { From 2d4e3bc4fd36fee9ade536ca05a63194fa146b7c Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Tue, 9 Jul 2024 09:56:11 +0200 Subject: [PATCH 08/18] v0.20.0 post-release. Signed-off-by: Gerd Zellweger --- Earthfile | 6 +++--- deploy/docker-compose-debezium.yml | 2 +- deploy/docker-compose.yml | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Earthfile b/Earthfile index e12967f5c1..902705f30f 100644 --- a/Earthfile +++ b/Earthfile @@ -372,13 +372,13 @@ test-docker-compose: test-docker-compose-stable: FROM earthly/dind:alpine COPY deploy/docker-compose.yml . - ENV FELDERA_VERSION=0.19.0 + ENV FELDERA_VERSION=0.20.0 RUN apk --no-cache add curl WITH DOCKER --pull postgres \ --pull docker.redpanda.com/vectorized/redpanda:v23.2.3 \ - --pull ghcr.io/feldera/pipeline-manager:0.19.0 \ + --pull ghcr.io/feldera/pipeline-manager:0.20.0 \ --load ghcr.io/feldera/pipeline-manager:latest=+build-pipeline-manager-container \ - --pull ghcr.io/feldera/demo-container:0.19.0 + --pull ghcr.io/feldera/demo-container:0.20.0 RUN COMPOSE_HTTP_TIMEOUT=120 SECOPS_DEMO_ARGS="--prepare-args 200000" RUST_LOG=debug,tokio_postgres=info docker-compose -f docker-compose.yml --profile demo up --force-recreate --exit-code-from demo && \ # This should run the latest version of the code and in the process, trigger a migration. COMPOSE_HTTP_TIMEOUT=120 SECOPS_DEMO_ARGS="--prepare-args 200000" FELDERA_VERSION=latest RUST_LOG=debug,tokio_postgres=info docker-compose -f docker-compose.yml up -d db pipeline-manager redpanda && \ diff --git a/deploy/docker-compose-debezium.yml b/deploy/docker-compose-debezium.yml index 1324b943d3..f029f5d4c8 100644 --- a/deploy/docker-compose-debezium.yml +++ b/deploy/docker-compose-debezium.yml @@ -28,7 +28,7 @@ services: condition: service_healthy mysql: condition: service_healthy - image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.19.0} + image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.20.0} environment: RUST_BACKTRACE: "1" REDPANDA_BROKERS: "redpanda:9092" diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 27b85c8a33..0013087082 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -2,7 +2,7 @@ name: feldera services: pipeline-manager: tty: true - image: ghcr.io/feldera/pipeline-manager:${FELDERA_VERSION:-0.19.0} + image: ghcr.io/feldera/pipeline-manager:${FELDERA_VERSION:-0.20.0} ports: # If you change the host side of the port mapping here, don't forget to # also add a corresponding --allowed-origins argument to the pipeline @@ -88,7 +88,7 @@ services: context: ../ dockerfile: deploy/Dockerfile target: kafka-connect - image: ghcr.io/feldera/kafka-connect:${FELDERA_VERSION:-0.19.0} + image: ghcr.io/feldera/kafka-connect:${FELDERA_VERSION:-0.20.0} depends_on: redpanda: condition: service_healthy @@ -118,7 +118,7 @@ services: condition: service_healthy redpanda: condition: service_healthy - image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.19.0} + image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.20.0} environment: - RUST_BACKTRACE=1 - REDPANDA_BROKERS=redpanda:9092 @@ -140,7 +140,7 @@ services: condition: service_healthy connect: condition: service_healthy - image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.19.0} + image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.20.0} environment: - RUST_BACKTRACE=1 - REDPANDA_BROKERS=redpanda:9092 @@ -177,7 +177,7 @@ services: condition: service_healthy redpanda: condition: service_healthy - image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.19.0} + image: ghcr.io/feldera/demo-container:${FELDERA_VERSION:-0.20.0} environment: - RUST_BACKTRACE=1 - RUST_LOG=info From c9689018fe30b2cd4e1222a08e50a86ce4f3b20b Mon Sep 17 00:00:00 2001 From: Simon Kassing Date: Tue, 9 Jul 2024 15:05:13 +0200 Subject: [PATCH 09/18] pipeline-manager: remove service probing - Remove API endpoints - Remove demo - Drop database table service_probe - Remove tests - Remove rdkafka dependency of pipeline-manager - Update OpenAPI spec - CHANGELOG entry Signed-off-by: Simon Kassing --- CHANGELOG.md | 4 + Cargo.lock | 1 - crates/pipeline-types/src/service/config.rs | 3 +- crates/pipeline_manager/Cargo.toml | 1 - .../migrations/V16__remove_service_probe.sql | 2 + crates/pipeline_manager/src/api/mod.rs | 21 +- crates/pipeline_manager/src/api/service.rs | 129 ------- .../src/bin/pipeline-manager.rs | 12 +- crates/pipeline_manager/src/config.rs | 60 --- crates/pipeline_manager/src/db/error.rs | 70 +--- crates/pipeline_manager/src/db/mod.rs | 194 ---------- crates/pipeline_manager/src/db/service.rs | 241 ------------ crates/pipeline_manager/src/db/storage.rs | 67 ---- crates/pipeline_manager/src/db/test.rs | 310 ---------------- .../pipeline_manager/src/integration_test.rs | 106 ------ crates/pipeline_manager/src/lib.rs | 1 - crates/pipeline_manager/src/prober/mod.rs | 124 ------- .../src/prober/service/kafka.rs | 96 ----- .../src/prober/service/mod.rs | 24 -- .../src/prober/service/probe.rs | 291 --------------- demo/service-probe/run.py | 73 ---- deploy/docker-compose.yml | 2 +- openapi.json | 347 +----------------- 23 files changed, 12 insertions(+), 2167 deletions(-) create mode 100644 crates/pipeline_manager/migrations/V16__remove_service_probe.sql delete mode 100644 crates/pipeline_manager/src/prober/mod.rs delete mode 100644 crates/pipeline_manager/src/prober/service/kafka.rs delete mode 100644 crates/pipeline_manager/src/prober/service/mod.rs delete mode 100644 crates/pipeline_manager/src/prober/service/probe.rs delete mode 100644 demo/service-probe/run.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a1bcdcb127..00b909f8b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- Service probing is removed: API endpoints and corresponding + demo are deleted, and the database table `service_probe` is dropped + ([#2002](https://github.com/feldera/feldera/pull/2002)) + ## [0.20.0] - 2024-07-09 - [SQL] Added `MATERIALIZED` views diff --git a/Cargo.lock b/Cargo.lock index 693af38d01..531390d4ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6953,7 +6953,6 @@ dependencies = [ "proptest", "proptest-derive 0.3.0", "rand 0.8.5", - "rdkafka", "refinery", "reqwest 0.11.27", "serde", diff --git a/crates/pipeline-types/src/service/config.rs b/crates/pipeline-types/src/service/config.rs index 3358020edf..8d77aff3d6 100644 --- a/crates/pipeline-types/src/service/config.rs +++ b/crates/pipeline-types/src/service/config.rs @@ -11,8 +11,7 @@ pub trait ServiceConfigVariant { /// Configuration for a Service, which typically includes how to establish a /// connection (e.g., hostname, port) and authenticate (e.g., credentials). /// -/// This configuration can be used to easily derive connectors for the service -/// as well as probe it for information. +/// This configuration can be used to easily derive connectors for the service. #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, ToSchema)] // snake_case such that the enumeration variants are not capitalized in (de-)serialization #[serde(rename_all = "snake_case")] diff --git a/crates/pipeline_manager/Cargo.toml b/crates/pipeline_manager/Cargo.toml index 27b9af6ea8..ca1dc9bf85 100644 --- a/crates/pipeline_manager/Cargo.toml +++ b/crates/pipeline_manager/Cargo.toml @@ -51,7 +51,6 @@ refinery = {version = "0.8.10", features = ["tokio-postgres"]} reqwest = {version = "0.11.18", features = ["json"]} url = {version = "2.4.0"} dirs = "5.0" -rdkafka = { version = "0.34.0", features = ["cmake-build", "ssl-vendored", "gssapi-vendored"] } thiserror = "1.0" metrics = "0.23" metrics-exporter-prometheus = "0.15.1" diff --git a/crates/pipeline_manager/migrations/V16__remove_service_probe.sql b/crates/pipeline_manager/migrations/V16__remove_service_probe.sql new file mode 100644 index 0000000000..7cb623d07f --- /dev/null +++ b/crates/pipeline_manager/migrations/V16__remove_service_probe.sql @@ -0,0 +1,2 @@ +-- Service probe mechanism is removed. +DROP TABLE IF EXISTS service_probe; diff --git a/crates/pipeline_manager/src/api/mod.rs b/crates/pipeline_manager/src/api/mod.rs index 544df684a1..668a2fd727 100644 --- a/crates/pipeline_manager/src/api/mod.rs +++ b/crates/pipeline_manager/src/api/mod.rs @@ -25,11 +25,6 @@ mod pipeline; mod program; mod service; -use crate::prober::service::{ - ServiceProbeError, ServiceProbeRequest, ServiceProbeResponse, ServiceProbeResult, - ServiceProbeStatus, ServiceProbeType, -}; - use crate::auth::JwkCache; use crate::compiler; use crate::config; @@ -57,8 +52,7 @@ use utoipa_swagger_ui::SwaggerUi; pub(crate) use crate::compiler::ProgramStatus; pub(crate) use crate::config::ApiServerConfig; use crate::db::{ - AttachedConnectorId, ConnectorId, PipelineId, ProgramId, ProjectDB, ServiceId, ServiceProbeId, - Version, + AttachedConnectorId, ConnectorId, PipelineId, ProgramId, ProjectDB, ServiceId, Version, }; pub use crate::error::ManagerError; use crate::runner::RunnerApi; @@ -151,8 +145,6 @@ request is rejected." service::new_service, service::update_service, service::delete_service, - service::new_service_probe, - service::list_service_probes, http_io::http_input, http_io::http_output, api_key::create_api_key, @@ -171,7 +163,6 @@ request is rejected." crate::db::ProgramDescr, crate::db::ConnectorDescr, crate::db::ServiceDescr, - crate::db::ServiceProbeDescr, crate::db::Pipeline, crate::db::PipelineRuntimeState, crate::db::PipelineDescr, @@ -237,7 +228,6 @@ request is rejected." ConnectorId, AttachedConnectorId, ServiceId, - ServiceProbeId, Version, ProgramStatus, ErrorResponse, @@ -266,15 +256,8 @@ request is rejected." service::UpdateServiceResponse, service::CreateOrReplaceServiceRequest, service::CreateOrReplaceServiceResponse, - service::CreateServiceProbeResponse, api_key::NewApiKeyRequest, api_key::NewApiKeyResponse, - ServiceProbeType, - ServiceProbeRequest, - ServiceProbeResponse, - ServiceProbeResult, - ServiceProbeError, - ServiceProbeStatus, compiler::ProgramConfig, config::CompilationProfile, pipeline_types_config::OutputBufferConfig, @@ -346,8 +329,6 @@ fn api_scope() -> Scope { .service(service::update_service) .service(service::create_or_replace_service) .service(service::delete_service) - .service(service::new_service_probe) - .service(service::list_service_probes) .service(api_key::create_api_key) .service(api_key::list_api_keys) .service(api_key::get_api_key) diff --git a/crates/pipeline_manager/src/api/service.rs b/crates/pipeline_manager/src/api/service.rs index 8c382110d4..efe9f4d4bf 100644 --- a/crates/pipeline_manager/src/api/service.rs +++ b/crates/pipeline_manager/src/api/service.rs @@ -2,14 +2,11 @@ /// which represent named external services such as Kafka. use super::{ManagerError, ServerState}; use crate::api::parse_string_param; -use crate::db::ServiceProbeId; -use crate::prober::service::{ServiceProbeRequest, ServiceProbeType}; use crate::{ api::examples, auth::TenantId, db::{storage::Storage, DBError, ServiceId}, }; -use actix_web::web::Query; use actix_web::{ delete, get, http::header::{CacheControl, CacheDirective}, @@ -17,7 +14,6 @@ use actix_web::{ web::{self, Data as WebData, ReqData}, HttpRequest, HttpResponse, }; -use chrono::Utc; use log::info; use pipeline_types::service::ServiceConfig; use serde::{Deserialize, Serialize}; @@ -89,27 +85,6 @@ pub(crate) struct CreateOrReplaceServiceResponse { service_id: ServiceId, } -/// Response to a create service probe request. -#[derive(Serialize, ToSchema)] -pub(crate) struct CreateServiceProbeResponse { - /// Unique id assigned to the service probe. - service_probe_id: ServiceProbeId, -} - -/// Request to retrieve a (limited) list of service probes, optionally filtered -/// by id. -#[derive(Debug, Deserialize, IntoParams)] -pub struct ListServiceProbes { - /// If provided, will filter based on exact match of the service probe - /// identifier. - id: Option, - /// If provided, will limit the amount of probes to the N most recent. - limit: Option, - /// If provided, will only have probes of that particular type. - #[serde(rename = "type")] - probe_type: Option, -} - /// Fetch services, optionally filtered by name, ID or configuration type. #[utoipa::path( responses( @@ -367,107 +342,3 @@ async fn get_service( .insert_header(CacheControl(vec![CacheDirective::NoCache])) .json(&descr)) } - -/// Create a service probe. -#[utoipa::path( - request_body = ServiceProbeRequest, - responses( - (status = CREATED, description = "Service probe created successfully", body = CreateServiceProbeResponse), - (status = NOT_FOUND - , description = "Specified service name does not exist" - , body = ErrorResponse - , example = json!(examples::unknown_name())), - ), - params( - ("service_name" = String, Path, description = "Unique service name") - ), - context_path = "/v0", - security(("JSON web token (JWT) or API key" = [])), - tag = "Services" -)] -#[post("/services/{service_name}/probes")] -async fn new_service_probe( - state: WebData, - tenant_id: ReqData, - request: HttpRequest, - body: web::Json, -) -> Result { - let service_name = parse_string_param(&request, "service_name")?; - let service_probe_id = state - .db - .lock() - .await - .new_service_probe_for_service_by_name( - *tenant_id, - &service_name, - Uuid::now_v7(), - body.0, - &Utc::now(), - ) - .await?; - info!( - "Created for service {} probe with id {} (tenant: {})", - service_name, service_probe_id, *tenant_id - ); - Ok(HttpResponse::Created() - .insert_header(CacheControl(vec![CacheDirective::NoCache])) - .json(&CreateServiceProbeResponse { service_probe_id })) -} - -/// Fetch a list of probes for a service, optionally filtered by id. -#[utoipa::path( - responses( - (status = OK, description = "Service probes retrieved successfully.", body = [ServiceProbeDescr]), - (status = NOT_FOUND - , description = "Specified service name does not exist" - , body = ErrorResponse - , example = json!(examples::unknown_name())) - ), - params( - ListServiceProbes, - ("service_name" = String, Path, description = "Unique service name"), - ), - context_path = "/v0", - security(("JSON web token (JWT) or API key" = [])), - tag = "Services" -)] -#[get("/services/{service_name}/probes")] -async fn list_service_probes( - state: WebData, - tenant_id: ReqData, - req: HttpRequest, - query: Query, -) -> Result { - let service_name = parse_string_param(&req, "service_name")?; - let descr = if let Some(id) = query.id { - // TODO: return error if query.limit is specified as well - vec![ - state - .db - .lock() - .await - .get_service_probe_for_service_by_name( - *tenant_id, - &service_name, - ServiceProbeId(id), - ) - .await?, - ] - } else { - state - .db - .lock() - .await - .list_service_probes_for_service_by_name( - *tenant_id, - &service_name, - query.limit, - query.probe_type.clone(), - ) - .await? - }; - - Ok(HttpResponse::Ok() - .insert_header(CacheControl(vec![CacheDirective::NoCache])) - .json(&descr)) -} diff --git a/crates/pipeline_manager/src/bin/pipeline-manager.rs b/crates/pipeline_manager/src/bin/pipeline-manager.rs index 244a568512..65e4e870be 100644 --- a/crates/pipeline_manager/src/bin/pipeline-manager.rs +++ b/crates/pipeline_manager/src/bin/pipeline-manager.rs @@ -6,11 +6,10 @@ use colored::Colorize; use pipeline_manager::api::ApiDoc; use pipeline_manager::compiler::Compiler; use pipeline_manager::config::{ - ApiServerConfig, CompilerConfig, DatabaseConfig, LocalRunnerConfig, ProberConfig, + ApiServerConfig, CompilerConfig, DatabaseConfig, LocalRunnerConfig, }; use pipeline_manager::db::ProjectDB; use pipeline_manager::local_runner; -use pipeline_manager::prober::run_prober; use std::sync::Arc; use tokio::sync::Mutex; use utoipa::OpenApi; @@ -28,7 +27,6 @@ async fn main() -> anyhow::Result<()> { let cli = ApiServerConfig::augment_args(cli); let cli = CompilerConfig::augment_args(cli); let cli = LocalRunnerConfig::augment_args(cli); - let cli = ProberConfig::augment_args(cli); let matches = cli.get_matches(); let mut api_config = ApiServerConfig::from_arg_matches(&matches) @@ -55,14 +53,10 @@ async fn main() -> anyhow::Result<()> { let local_runner_config = LocalRunnerConfig::from_arg_matches(&matches) .map_err(|err| err.exit()) .unwrap(); - let prober_config = ProberConfig::from_arg_matches(&matches) - .map_err(|err| err.exit()) - .unwrap(); let api_config = api_config.canonicalize()?; let compiler_config = compiler_config.canonicalize()?; let local_runner_config = local_runner_config.canonicalize()?; - let prober_config = prober_config.canonicalize()?; let metrics_handle = pipeline_manager::metrics::init(); if compiler_config.precompile { @@ -92,10 +86,6 @@ async fn main() -> anyhow::Result<()> { let _local_runner = tokio::spawn(async move { local_runner::run(db_clone, &local_runner_config).await; }); - let db_clone = db.clone(); - let _prober = tokio::spawn(async move { - run_prober(&prober_config, db_clone).await.unwrap(); - }); pipeline_manager::metrics::create_endpoint(metrics_handle, db.clone()).await; // The api-server blocks forever pipeline_manager::api::run(db, api_config).await.unwrap(); diff --git a/crates/pipeline_manager/src/config.rs b/crates/pipeline_manager/src/config.rs index a0eb2558f9..f43f896dd9 100644 --- a/crates/pipeline_manager/src/config.rs +++ b/crates/pipeline_manager/src/config.rs @@ -586,63 +586,3 @@ impl LocalRunnerConfig { .join(pipeline_types::transport::http::SERVER_PORT_FILE) } } - -fn default_prober_web_server_port() -> u16 { - 44444 -} - -fn default_probe_sleep_inbetween_ms() -> u64 { - 1000 -} - -fn default_probe_timeout_ms() -> u64 { - 1500 -} - -/// Prober configuration. -/// This configuration is read (deserialized) from a YAML config file -/// or parsed from command-line arguments. -#[derive(Parser, Deserialize, Debug, Clone)] -pub struct ProberConfig { - /// General working directory where the prober will store its logs. - #[serde(default = "default_working_directory")] - #[arg(long, default_value_t = default_working_directory())] - pub prober_working_directory: String, - - /// The port used by the web server of the prober. - #[arg(long, default_value_t = default_prober_web_server_port())] - pub prober_http_server_port: u16, - - /// Duration (ms) slept between checks when no probes to perform are - /// available. - #[arg(long, default_value_t = default_probe_sleep_inbetween_ms())] - pub probe_sleep_inbetween_ms: u64, - - /// Probe timeout (ms). - #[arg(long, default_value_t = default_probe_timeout_ms())] - pub probe_timeout_ms: u64, -} - -impl ProberConfig { - /// Convert all directory paths in the `self` to absolute paths. - pub fn canonicalize(mut self) -> AnyResult { - create_dir_all(&self.prober_working_directory).map_err(|e| { - AnyError::msg(format!( - "unable to create or open directory '{}': {e}", - self.prober_working_directory - )) - })?; - - self.prober_working_directory = canonicalize(&self.prober_working_directory) - .map_err(|e| { - AnyError::msg(format!( - "error canonicalizing directory path '{}': {e}", - self.prober_working_directory - )) - })? - .to_string_lossy() - .into_owned(); - - Ok(self) - } -} diff --git a/crates/pipeline_manager/src/db/error.rs b/crates/pipeline_manager/src/db/error.rs index 7d63549ca6..6efa39a8e5 100644 --- a/crates/pipeline_manager/src/db/error.rs +++ b/crates/pipeline_manager/src/db/error.rs @@ -1,6 +1,6 @@ use super::{ConnectorId, PipelineId, ProgramId, Version}; use crate::auth::TenantId; -use crate::db::{ServiceId, ServiceProbeId}; +use crate::db::ServiceId; use actix_web::{ body::BoxBody, http::StatusCode, HttpResponse, HttpResponseBuilder, ResponseError, }; @@ -82,9 +82,6 @@ pub enum DBError { UnknownServiceName { service_name: String, }, - UnknownServiceProbe { - service_probe_id: ServiceProbeId, - }, UnknownApiKey { name: String, }, @@ -114,16 +111,6 @@ pub enum DBError { status: String, backtrace: Backtrace, }, - #[serde(serialize_with = "serialize_unknown_service_probe_status")] - UnknownServiceProbeStatus { - status: String, - backtrace: Backtrace, - }, - #[serde(serialize_with = "serialize_unknown_service_probe_type")] - UnknownServiceProbeType { - probe_type: String, - backtrace: Backtrace, - }, ProgramNotSet, ProgramNotCompiled, ProgramFailedToCompile, @@ -162,18 +149,6 @@ impl DBError { backtrace: Backtrace::capture(), } } - pub fn unknown_service_probe_status(status: String) -> Self { - Self::UnknownServiceProbeStatus { - status, - backtrace: Backtrace::capture(), - } - } - pub fn unknown_service_probe_type(probe_type: String) -> Self { - Self::UnknownServiceProbeType { - probe_type, - backtrace: Backtrace::capture(), - } - } pub fn unique_key_violation(constraint: &'static str) -> Self { Self::UniqueKeyViolation { constraint, @@ -309,34 +284,6 @@ where ser.end() } -fn serialize_unknown_service_probe_status( - status: &String, - backtrace: &Backtrace, - serializer: S, -) -> Result -where - S: Serializer, -{ - let mut ser = serializer.serialize_struct("UnknownServiceProbeStatus", 2)?; - ser.serialize_field("status", &status.to_string())?; - ser.serialize_field("backtrace", &backtrace.to_string())?; - ser.end() -} - -fn serialize_unknown_service_probe_type( - probe_type: &String, - backtrace: &Backtrace, - serializer: S, -) -> Result -where - S: Serializer, -{ - let mut ser = serializer.serialize_struct("UnknownServiceProbeType", 2)?; - ser.serialize_field("probe_type", &probe_type.to_string())?; - ser.serialize_field("backtrace", &backtrace.to_string())?; - ser.end() -} - impl From for DBError { fn from(error: PgError) -> Self { Self::PostgresError { @@ -438,9 +385,6 @@ impl Display for DBError { DBError::UnknownServiceName { service_name } => { write!(f, "Unknown service name '{service_name}'") } - DBError::UnknownServiceProbe { service_probe_id } => { - write!(f, "Unknown service probe id '{service_probe_id}'") - } DBError::UnknownApiKey { name } => { write!(f, "Unknown API key '{name}'") } @@ -465,12 +409,6 @@ impl Display for DBError { DBError::UnknownPipelineStatus { status, .. } => { write!(f, "Unknown pipeline status '{status}' encountered") } - DBError::UnknownServiceProbeStatus { status, .. } => { - write!(f, "Unknown service probe status '{status}' encountered") - } - DBError::UnknownServiceProbeType { probe_type, .. } => { - write!(f, "Unknown service probe type '{probe_type}' encountered") - } DBError::ProgramNotSet => write!(f, "The pipeline does not have a program attached"), DBError::ProgramNotCompiled => { write!( @@ -538,7 +476,6 @@ impl DetailedError for DBError { Self::InvalidConnectorTransport { .. } => Cow::from("InvalidConnectorTransport"), Self::UnknownService { .. } => Cow::from("UnknownService"), Self::UnknownServiceName { .. } => Cow::from("UnknownServiceName"), - Self::UnknownServiceProbe { .. } => Cow::from("UnknownServiceProbe"), Self::UnknownApiKey { .. } => Cow::from("UnknownApiKey"), Self::UnknownTenant { .. } => Cow::from("UnknownTenant"), Self::UnknownAttachedConnector { .. } => Cow::from("UnknownAttachedConnector"), @@ -548,8 +485,6 @@ impl DetailedError for DBError { Self::InvalidKey => Cow::from("InvalidKey"), Self::UniqueKeyViolation { .. } => Cow::from("UniqueKeyViolation"), Self::UnknownPipelineStatus { .. } => Cow::from("UnknownPipelineStatus"), - Self::UnknownServiceProbeStatus { .. } => Cow::from("UnknownServiceProbeStatus"), - Self::UnknownServiceProbeType { .. } => Cow::from("UnknownServiceProbeType"), Self::ProgramNotSet => Cow::from("ProgramNotSet"), Self::ProgramNotCompiled => Cow::from("ProgramNotCompiled"), Self::ProgramFailedToCompile => Cow::from("ProgramFailedToCompile"), @@ -598,7 +533,6 @@ impl ResponseError for DBError { Self::InvalidConnectorTransport { .. } => StatusCode::BAD_REQUEST, Self::UnknownService { .. } => StatusCode::NOT_FOUND, Self::UnknownServiceName { .. } => StatusCode::NOT_FOUND, - Self::UnknownServiceProbe { .. } => StatusCode::NOT_FOUND, Self::UnknownApiKey { .. } => StatusCode::NOT_FOUND, // TODO: should we report not found instead? Self::UnknownTenant { .. } => StatusCode::UNAUTHORIZED, @@ -615,8 +549,6 @@ impl ResponseError for DBError { Self::MissingMigrations { .. } => StatusCode::INTERNAL_SERVER_ERROR, // should in practice not happen, e.g., would mean invalid status in db: Self::UnknownPipelineStatus { .. } => StatusCode::INTERNAL_SERVER_ERROR, - Self::UnknownServiceProbeStatus { .. } => StatusCode::INTERNAL_SERVER_ERROR, - Self::UnknownServiceProbeType { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::NoRevisionAvailable { .. } => StatusCode::NOT_FOUND, Self::RevisionNotChanged => StatusCode::BAD_REQUEST, Self::TablesNotInSchema { .. } => StatusCode::BAD_REQUEST, diff --git a/crates/pipeline_manager/src/db/mod.rs b/crates/pipeline_manager/src/db/mod.rs index 407c9495e9..96730ba22a 100644 --- a/crates/pipeline_manager/src/db/mod.rs +++ b/crates/pipeline_manager/src/db/mod.rs @@ -6,7 +6,6 @@ use crate::{ config::DatabaseConfig, }; use async_trait::async_trait; -use chrono::{DateTime, Utc}; use deadpool_postgres::{Manager, Pool, RecyclingMethod, Transaction}; use log::{debug, info}; use openssl::sha; @@ -29,8 +28,6 @@ mod pg_setup; pub(crate) mod storage; mod error; -pub(crate) use crate::db::service::{ServiceProbeDescr, ServiceProbeId}; -use crate::prober::service::{ServiceProbeRequest, ServiceProbeResponse, ServiceProbeType}; pub use error::DBError; use pipeline_types::service::ServiceConfig; @@ -825,86 +822,6 @@ impl Storage for ProjectDB { async fn delete_service(&self, tenant_id: TenantId, service_name: &str) -> Result<(), DBError> { Ok(service::delete_service(self, tenant_id, service_name).await?) } - - async fn new_service_probe( - &self, - tenant_id: TenantId, - service_id: ServiceId, - id: Uuid, - request: ServiceProbeRequest, - created_at: &DateTime, - txn: Option<&Transaction<'_>>, - ) -> Result { - Ok( - service::new_service_probe(self, tenant_id, service_id, id, request, created_at, txn) - .await?, - ) - } - - async fn update_service_probe_set_running( - &self, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - started_at: &DateTime, - ) -> Result<(), DBError> { - Ok( - service::update_service_probe_set_running( - self, - tenant_id, - service_probe_id, - started_at, - ) - .await?, - ) - } - - async fn update_service_probe_set_finished( - &self, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - response: ServiceProbeResponse, - finished_at: &DateTime, - ) -> Result<(), DBError> { - Ok(service::update_service_probe_set_finished( - self, - tenant_id, - service_probe_id, - response, - finished_at, - ) - .await?) - } - - async fn next_service_probe( - &self, - ) -> Result, DBError> - { - Ok(service::next_service_probe(self).await?) - } - - async fn get_service_probe( - &self, - tenant_id: TenantId, - service_id: ServiceId, - service_probe_id: ServiceProbeId, - txn: Option<&Transaction<'_>>, - ) -> Result { - Ok(service::get_service_probe(self, tenant_id, service_id, service_probe_id, txn).await?) - } - - async fn list_service_probes( - &self, - tenant_id: TenantId, - service_id: ServiceId, - limit: Option, - probe_type: Option, - txn: Option<&Transaction<'_>>, - ) -> Result, DBError> { - Ok( - service::list_service_probes(self, tenant_id, service_id, limit, probe_type, txn) - .await?, - ) - } } impl ProjectDB { @@ -1023,9 +940,6 @@ impl ProjectDB { Some("program_pkey") => DBError::unique_key_violation("program_pkey"), Some("connector_pkey") => DBError::unique_key_violation("connector_pkey"), Some("service_pkey") => DBError::unique_key_violation("service_pkey"), - Some("service_probe_pkey") => { - DBError::unique_key_violation("service_probe_pkey") - } Some("pipeline_pkey") => DBError::unique_key_violation("pipeline_pkey"), Some("api_key_pkey") => DBError::unique_key_violation("api_key_pkey"), Some("unique_hash") => DBError::duplicate_key(), @@ -1139,46 +1053,6 @@ impl ProjectDB { err } - /// Helper to convert service_id foreign key constraint error into an - /// user-friendly error message. - fn maybe_service_id_foreign_key_constraint_err(err: DBError, service_id: ServiceId) -> DBError { - if let DBError::PostgresError { error, .. } = &err { - let db_err = error.as_db_error(); - if let Some(db_err) = db_err { - if db_err.code() == &tokio_postgres::error::SqlState::FOREIGN_KEY_VIOLATION - && (db_err.constraint() == Some("service_service_id_fkey") - || db_err.constraint() == Some("service_probe_service_id_fkey")) - { - return DBError::UnknownService { service_id }; - } - } - } - - err - } - - /// Helper to convert tenant_id_service_id foreign key constraint error into - /// an user-friendly error message. - fn maybe_tenant_id_service_id_foreign_key_constraint_err( - err: DBError, - service_id: ServiceId, - ) -> DBError { - if let DBError::PostgresError { error, .. } = &err { - let db_err = error.as_db_error(); - if let Some(db_err) = db_err { - if db_err.code() == &tokio_postgres::error::SqlState::FOREIGN_KEY_VIOLATION - && db_err.constraint() == Some("service_probe_tenant_id_service_id_fkey") - { - // TODO: specialized error indicating the service specifically does not exist - // for this tenant? - return DBError::UnknownService { service_id }; - } - } - } - - err - } - // TODO: Should be part of the Storage trait pub(crate) async fn pipeline_config( &self, @@ -1530,72 +1404,4 @@ impl ProjectDB { txn.commit().await?; Ok(()) } - - /// Creates a new service probe for the service named `service_name`. - pub async fn new_service_probe_for_service_by_name( - &self, - tenant_id: TenantId, - service_name: &str, - id: Uuid, - request: ServiceProbeRequest, - created_at: &DateTime, - ) -> Result { - let mut manager = self.pool.get().await?; - let txn = manager.transaction().await?; - let service = self - .get_service_by_name(tenant_id, service_name, Some(&txn)) - .await?; - let service_probe_id = self - .new_service_probe( - tenant_id, - service.service_id, - id, - request, - created_at, - Some(&txn), - ) - .await?; - txn.commit().await?; - Ok(service_probe_id) - } - - /// Retrieves the service probe with id `service_probe_id` - /// for the service named `service_name`. - pub(crate) async fn get_service_probe_for_service_by_name( - &self, - tenant_id: TenantId, - service_name: &str, - service_probe_id: ServiceProbeId, - ) -> Result { - let mut manager = self.pool.get().await?; - let txn = manager.transaction().await?; - let service = self - .get_service_by_name(tenant_id, service_name, Some(&txn)) - .await?; - let descr = self - .get_service_probe(tenant_id, service.service_id, service_probe_id, Some(&txn)) - .await?; - txn.commit().await?; - Ok(descr) - } - - /// Retrieves the list of service probes for the service named `service_name`. - pub(crate) async fn list_service_probes_for_service_by_name( - &self, - tenant_id: TenantId, - service_name: &str, - limit: Option, - probe_type: Option, - ) -> Result, DBError> { - let mut manager = self.pool.get().await?; - let txn = manager.transaction().await?; - let service = self - .get_service_by_name(tenant_id, service_name, Some(&txn)) - .await?; - let descr_list = self - .list_service_probes(tenant_id, service.service_id, limit, probe_type, Some(&txn)) - .await?; - txn.commit().await?; - Ok(descr_list) - } } diff --git a/crates/pipeline_manager/src/db/service.rs b/crates/pipeline_manager/src/db/service.rs index 6fb6a008bb..a34004fcc9 100644 --- a/crates/pipeline_manager/src/db/service.rs +++ b/crates/pipeline_manager/src/db/service.rs @@ -1,10 +1,5 @@ use crate::auth::TenantId; -use crate::db::pipeline::convert_bigint_to_time; use crate::db::{DBError, ProjectDB}; -use crate::prober::service::{ - ServiceProbeRequest, ServiceProbeResponse, ServiceProbeStatus, ServiceProbeType, -}; -use chrono::{DateTime, Utc}; use deadpool_postgres::Transaction; use log::debug; use pipeline_types::service::ServiceConfig; @@ -12,7 +7,6 @@ use serde::{Deserialize, Serialize}; use std::fmt; use std::fmt::Display; use tokio_postgres::types::ToSql; -use tokio_postgres::Row; use utoipa::ToSchema; use uuid::Uuid; @@ -30,20 +24,6 @@ impl Display for ServiceId { } } -/// Unique service probe id. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Ord, PartialOrd, Serialize, Deserialize, ToSchema)] -#[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[repr(transparent)] -#[serde(transparent)] -pub struct ServiceProbeId( - #[cfg_attr(test, proptest(strategy = "super::test::limited_uuid()"))] pub Uuid, -); -impl Display for ServiceProbeId { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) - } -} - /// Service descriptor. #[derive(Deserialize, Serialize, ToSchema, Debug, Clone, Eq, PartialEq)] pub(crate) struct ServiceDescr { @@ -54,19 +34,6 @@ pub(crate) struct ServiceDescr { pub config_type: String, } -/// Service probe descriptor. -#[derive(Deserialize, Serialize, ToSchema, Debug, Clone, Eq, PartialEq)] -pub(crate) struct ServiceProbeDescr { - pub service_probe_id: ServiceProbeId, - pub status: ServiceProbeStatus, - pub probe_type: ServiceProbeType, - pub request: ServiceProbeRequest, - pub response: Option, - pub created_at: DateTime, - pub started_at: Option>, - pub finished_at: Option>, -} - pub(crate) async fn new_service( db: &ProjectDB, tenant_id: TenantId, @@ -261,211 +228,3 @@ pub(crate) async fn delete_service( }) } } - -pub(crate) async fn new_service_probe( - db: &ProjectDB, - tenant_id: TenantId, - service_id: ServiceId, - id: Uuid, - request: ServiceProbeRequest, - created_at: &DateTime, - txn: Option<&Transaction<'_>>, -) -> Result { - debug!("new_service_probe {id}"); - let query = r#"INSERT INTO service_probe - (id, tenant_id, service_id, status, request, probe_type, - response, created_at, started_at, finished_at) - VALUES ($1, $2, $3, $4, $5, $6, NULL, $7, NULL, NULL)"#; - let status: &'static str = ServiceProbeStatus::Pending.into(); - let probe_type: &'static str = request.probe_type().into(); - let params: &[&(dyn ToSql + Sync)] = &[ - &id, - &tenant_id.0, - &service_id.0, - &status, - &request.to_yaml(), - &probe_type, - &created_at.timestamp(), - ]; - - if let Some(txn) = txn { - let stmt = txn.prepare_cached(query).await?; - txn.execute(&stmt, params).await - } else { - let manager = db.pool.get().await?; - let stmt = manager.prepare_cached(query).await?; - manager.execute(&stmt, params).await - } - .map_err(ProjectDB::maybe_unique_violation) - .map_err(|e| ProjectDB::maybe_tenant_id_service_id_foreign_key_constraint_err(e, service_id)) - .map_err(|e| ProjectDB::maybe_service_id_foreign_key_constraint_err(e, service_id)) - .map_err(|e| ProjectDB::maybe_tenant_id_foreign_key_constraint_err(e, tenant_id, None))?; - Ok(ServiceProbeId(id)) -} - -pub(crate) async fn update_service_probe_set_running( - db: &ProjectDB, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - started_at: &DateTime, -) -> Result<(), DBError> { - let query = - "UPDATE service_probe SET status = $1, started_at = $2 WHERE tenant_id = $3 AND id = $4 "; - let status: &'static str = ServiceProbeStatus::Running.into(); - let params: &[&(dyn ToSql + Sync)] = &[ - &status, - &started_at.timestamp(), - &tenant_id.0, - &service_probe_id.0, - ]; - - let manager = db.pool.get().await?; - let stmt = manager.prepare_cached(query).await?; - let modified_rows = manager.execute(&stmt, params).await?; - if modified_rows > 0 { - Ok(()) - } else { - Err(DBError::UnknownServiceProbe { service_probe_id }) - } -} - -pub(crate) async fn update_service_probe_set_finished( - db: &ProjectDB, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - response: ServiceProbeResponse, - finished_at: &DateTime, -) -> Result<(), DBError> { - let query = "UPDATE service_probe SET status = $1, response = $2, finished_at = $3 WHERE tenant_id = $4 AND id = $5 "; - let status: &'static str = match response { - ServiceProbeResponse::Success(_) => ServiceProbeStatus::Success.into(), - ServiceProbeResponse::Error(_) => ServiceProbeStatus::Failure.into(), - }; - let params: &[&(dyn ToSql + Sync)] = &[ - &status, - &response.to_yaml(), - &finished_at.timestamp(), - &tenant_id.0, - &service_probe_id.0, - ]; - - let manager = db.pool.get().await?; - let stmt = manager.prepare_cached(query).await?; - let modified_rows = manager.execute(&stmt, params).await?; - if modified_rows > 0 { - Ok(()) - } else { - Err(DBError::UnknownServiceProbe { service_probe_id }) - } -} - -pub(crate) async fn next_service_probe( - db: &ProjectDB, -) -> Result, DBError> { - let manager = db.pool.get().await?; - let query = r#"SELECT sp.id, sp.tenant_id, sp.request, sr.config - FROM service_probe sp - LEFT JOIN service sr ON sp.service_id = sr.id - WHERE sp.status = 'pending' OR sp.status = 'running' - ORDER BY (sp.created_at, sp.id) ASC LIMIT 1"#; - let stmt = manager.prepare_cached(query).await?; - let row = manager.query_opt(&stmt, &[]).await?; - if let Some(row) = row { - Ok(Some(( - ServiceProbeId(row.get(0)), - TenantId(row.get(1)), - ServiceProbeRequest::from_yaml_str(row.get(2)), - ServiceConfig::from_yaml_str(row.get(3)), - ))) - } else { - Ok(None) - } -} - -/// Converts row of the service_probe table from a query result to its -/// descriptor. -fn service_probe_row_to_descriptor(row: Row) -> Result { - Ok(ServiceProbeDescr { - service_probe_id: ServiceProbeId(row.get(0)), - status: row.get::<_, String>(1).try_into()?, - request: ServiceProbeRequest::from_yaml_str(row.get(2)), - probe_type: row.get::<_, String>(3).try_into()?, - response: row - .get::>(4) - .map(ServiceProbeResponse::from_yaml_str), - created_at: convert_bigint_to_time("service_probe.created_at", row.get(5))?, - started_at: match row.get(6) { - None => None, - Some(v) => Some(convert_bigint_to_time("service_probe.started_at", v)?), - }, - finished_at: match row.get(7) { - None => None, - Some(v) => Some(convert_bigint_to_time("service_probe.finished_at", v)?), - }, - }) -} - -pub(crate) async fn get_service_probe( - db: &ProjectDB, - tenant_id: TenantId, - service_id: ServiceId, - service_probe_id: ServiceProbeId, - txn: Option<&Transaction<'_>>, -) -> Result { - let query = r#"SELECT sp.id, sp.status, sp.request, sp.probe_type, sp.response, sp.created_at, sp.started_at, sp.finished_at - FROM service_probe sp - WHERE sp.tenant_id = $1 AND sp.service_id = $2 AND sp.id = $3"#; - let row = if let Some(txn) = txn { - let stmt = txn.prepare_cached(query).await?; - txn.query_opt(&stmt, &[&tenant_id.0, &service_id.0, &service_probe_id.0]) - .await? - } else { - let manager = db.pool.get().await?; - let stmt = manager.prepare_cached(query).await?; - manager - .query_opt(&stmt, &[&tenant_id.0, &service_id.0, &service_probe_id.0]) - .await? - }; - - if let Some(row) = row { - Ok(service_probe_row_to_descriptor(row)?) - } else { - Err(DBError::UnknownServiceProbe { service_probe_id }) - } -} - -pub(crate) async fn list_service_probes( - db: &ProjectDB, - tenant_id: TenantId, - service_id: ServiceId, - limit: Option, - probe_type: Option, - txn: Option<&Transaction<'_>>, -) -> Result, DBError> { - let query = r#"SELECT sp.id, sp.status, sp.request, sp.probe_type, sp.response, sp.created_at, sp.started_at, sp.finished_at - FROM service_probe sp - WHERE sp.tenant_id = $1 AND sp.service_id = $2 - AND sp.probe_type = COALESCE($3, sp.probe_type) - ORDER BY (sp.created_at, sp.id) DESC - LIMIT $4"#; // NULL is the same as ALL - let probe_type: Option<&'static str> = probe_type.map(|t| t.into()); - let params: &[&(dyn ToSql + Sync)] = &[ - &tenant_id.0, - &service_id.0, - &probe_type, - &(limit.map(|v| v as i64)), - ]; - let rows = if let Some(txn) = txn { - let stmt = txn.prepare_cached(query).await?; - txn.query(&stmt, params).await? - } else { - let manager = db.pool.get().await?; - let stmt = manager.prepare_cached(query).await?; - manager.query(&stmt, params).await? - }; - let mut result = Vec::with_capacity(rows.len()); - for row in rows { - result.push(service_probe_row_to_descriptor(row)?); - } - Ok(result) -} diff --git a/crates/pipeline_manager/src/db/storage.rs b/crates/pipeline_manager/src/db/storage.rs index 7fe6876fca..a64971bcf2 100644 --- a/crates/pipeline_manager/src/db/storage.rs +++ b/crates/pipeline_manager/src/db/storage.rs @@ -6,11 +6,8 @@ use super::{ use crate::api::ProgramStatus; use crate::auth::TenantId; use crate::compiler::ProgramConfig; -use crate::db::service::{ServiceProbeDescr, ServiceProbeId}; use crate::db::{ServiceDescr, ServiceId}; -use crate::prober::service::{ServiceProbeRequest, ServiceProbeResponse, ServiceProbeType}; use async_trait::async_trait; -use chrono::{DateTime, Utc}; use deadpool_postgres::Transaction; use pipeline_types::config::ConnectorConfig; use pipeline_types::service::ServiceConfig; @@ -452,70 +449,6 @@ pub(crate) trait Storage { /// Returns error if there does not exist a service with the provided name. async fn delete_service(&self, tenant_id: TenantId, service_name: &str) -> Result<(), DBError>; - /// Creates a new service probe. - /// - /// Returns error if the tenant or service referenced by identifier does not - /// exist. - async fn new_service_probe( - &self, - tenant_id: TenantId, - service_id: ServiceId, - id: Uuid, - request: ServiceProbeRequest, - created_at: &DateTime, - txn: Option<&Transaction<'_>>, - ) -> Result; - - /// Sets the status of an existing service probe to running and saves the - /// starting timestamp. - /// - /// Returns error if the service_probe_id does not exist. - async fn update_service_probe_set_running( - &self, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - started_at: &DateTime, - ) -> Result<(), DBError>; - - /// Sets the status of an existing service probe to either succeeded or - /// failed based on the success,and saves the response and finishing - /// timestamp. - /// - /// Returns error if the service_probe_id does not exist. - async fn update_service_probe_set_finished( - &self, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - response: ServiceProbeResponse, - finished_at: &DateTime, - ) -> Result<(), DBError>; - - /// Retrieves the next service probe to perform. - async fn next_service_probe( - &self, - ) -> Result, DBError>; - - /// Retrieves service probe by its identifier. - async fn get_service_probe( - &self, - tenant_id: TenantId, - service_id: ServiceId, - service_probe_id: ServiceProbeId, - txn: Option<&Transaction<'_>>, - ) -> Result; - - /// Retrieves a list of all probes of a service. - /// Optionally, limited to a specific number of the most recent probes. - /// Optionally, filtered to a specific request type. - async fn list_service_probes( - &self, - tenant_id: TenantId, - service_id: ServiceId, - limit: Option, - probe_type: Option, - txn: Option<&Transaction<'_>>, - ) -> Result, DBError>; - /// Check connectivity to the DB async fn check_connection(&self) -> Result<(), DBError>; } diff --git a/crates/pipeline_manager/src/db/test.rs b/crates/pipeline_manager/src/db/test.rs index bde6b33114..5539b642ce 100644 --- a/crates/pipeline_manager/src/db/test.rs +++ b/crates/pipeline_manager/src/db/test.rs @@ -10,12 +10,7 @@ use super::{ use crate::auth::{self, TenantId, TenantRecord}; use crate::compiler::ProgramConfig; use crate::config::CompilationProfile; -use crate::db::pipeline::convert_bigint_to_time; -use crate::db::service::{ServiceProbeDescr, ServiceProbeId}; use crate::db::{ServiceDescr, ServiceId}; -use crate::prober::service::{ - ServiceProbeRequest, ServiceProbeResponse, ServiceProbeStatus, ServiceProbeType, -}; use async_trait::async_trait; use chrono::{DateTime, NaiveDateTime, Utc}; use deadpool_postgres::Transaction; @@ -1253,25 +1248,6 @@ pub(crate) fn limited_option_service_config() -> impl Strategy impl Strategy> { - any::>().prop_map(|byte| byte.map(|b| (b % 5) as u32)) -} - -/// Generate an optional service probe request type including possibly invalid -pub(crate) fn limited_option_service_probe_type() -> impl Strategy> -{ - any::>().prop_map(|byte| { - byte.map(|b| { - if b <= 127 { - ServiceProbeRequest::TestConnectivity.probe_type() - } else { - ServiceProbeRequest::KafkaGetTopics.probe_type() - } - }) - }) -} - /// Generate different resource configurations pub(crate) fn runtime_config() -> impl Strategy { any::<( @@ -1468,32 +1444,6 @@ enum StorageAction { #[proptest(strategy = "limited_option_service_config()")] Option, ), DeleteService(TenantId, String), - NewServiceProbe( - TenantId, - ServiceId, - #[proptest(strategy = "limited_uuid()")] Uuid, - ServiceProbeRequest, - #[cfg_attr(test, proptest(value = "Utc::now()"))] DateTime, - ), - UpdateServiceProbeSetRunning( - TenantId, - ServiceProbeId, - #[cfg_attr(test, proptest(value = "Utc::now()"))] DateTime, - ), - UpdateServiceProbeSetFinished( - TenantId, - ServiceProbeId, - ServiceProbeResponse, - #[cfg_attr(test, proptest(value = "Utc::now()"))] DateTime, - ), - NextServiceProbe, - GetServiceProbe(TenantId, ServiceId, ServiceProbeId), - ListServiceProbes( - TenantId, - ServiceId, - #[proptest(strategy = "limited_option_list_limit()")] Option, - #[proptest(strategy = "limited_option_service_probe_type()")] Option, - ), ListApiKeys(TenantId), GetApiKey(TenantId, String), DeleteApiKey(TenantId, String), @@ -1977,41 +1927,6 @@ fn db_impl_behaves_like_model() { let impl_response = handle.db.delete_service(tenant_id, &service_name).await; check_responses(i, model_response, impl_response); } - StorageAction::NewServiceProbe(tenant_id, service_id, id, request, created_at) => { - create_tenants_if_not_exists(&model, &handle, tenant_id).await.unwrap(); - let model_response = model.new_service_probe(tenant_id, service_id, id, request.clone(), &created_at, None).await; - let impl_response = handle.db.new_service_probe(tenant_id, service_id, id, request.clone(), &created_at, None).await; - check_responses(i, model_response, impl_response); - } - StorageAction::UpdateServiceProbeSetRunning(tenant_id, service_probe_id, started_at) => { - create_tenants_if_not_exists(&model, &handle, tenant_id).await.unwrap(); - let model_response = model.update_service_probe_set_running(tenant_id, service_probe_id, &started_at).await; - let impl_response = handle.db.update_service_probe_set_running(tenant_id, service_probe_id, &started_at).await; - check_responses(i, model_response, impl_response); - } - StorageAction::UpdateServiceProbeSetFinished(tenant_id, service_probe_id, response, finished_at) => { - create_tenants_if_not_exists(&model, &handle, tenant_id).await.unwrap(); - let model_response = model.update_service_probe_set_finished(tenant_id, service_probe_id, response.clone(), &finished_at).await; - let impl_response = handle.db.update_service_probe_set_finished(tenant_id, service_probe_id, response.clone(), &finished_at).await; - check_responses(i, model_response, impl_response); - } - StorageAction::NextServiceProbe => { - let model_response = model.next_service_probe().await; - let impl_response = handle.db.next_service_probe().await; - check_responses(i, model_response, impl_response); - } - StorageAction::GetServiceProbe(tenant_id, service_id, service_probe_id) => { - create_tenants_if_not_exists(&model, &handle, tenant_id).await.unwrap(); - let model_response = model.get_service_probe(tenant_id, service_id, service_probe_id, None).await; - let impl_response = handle.db.get_service_probe(tenant_id, service_id, service_probe_id, None).await; - check_responses(i, model_response, impl_response); - } - StorageAction::ListServiceProbes(tenant_id, service_id, limit, probe_type) => { - create_tenants_if_not_exists(&model, &handle, tenant_id).await.unwrap(); - let model_response = model.list_service_probes(tenant_id, service_id, limit, probe_type.clone(), None).await; - let impl_response = handle.db.list_service_probes(tenant_id, service_id, limit, probe_type.clone(), None).await; - check_responses(i, model_response, impl_response); - } } } }); @@ -2037,7 +1952,6 @@ struct DbModel { pub api_keys: BTreeMap<(TenantId, String), (ApiKeyId, String, Vec)>, pub connectors: BTreeMap<(TenantId, ConnectorId), ConnectorDescr>, pub services: BTreeMap<(TenantId, ServiceId), ServiceDescr>, - pub service_probes: BTreeMap<(TenantId, ServiceProbeId), (ServiceProbeDescr, ServiceId)>, pub tenants: BTreeMap, } @@ -3227,9 +3141,6 @@ impl Storage for Mutex { })?; let service_id = service_id.clone(); - // Cascade: remove any service probes of the service - s.service_probes.retain(|_, (_, id)| *id != service_id); - // Find all connectors which have the service let connectors_with_the_service: Vec = s .connectors @@ -3267,225 +3178,4 @@ impl Storage for Mutex { Ok(()) } - - async fn new_service_probe( - &self, - tenant_id: TenantId, - service_id: ServiceId, - id: Uuid, - request: ServiceProbeRequest, - created_at: &DateTime, - _txn: Option<&Transaction<'_>>, - ) -> Result { - let mut s = self.lock().await; - let service_probe_id = ServiceProbeId(id); - - // Primary key constraint violation - if s.service_probes.keys().any(|k| k.1 == service_probe_id) { - return Err(DBError::unique_key_violation("service_probe_pkey")); - } - - // Service must exist - if !s - .services - .keys() - .any(|k| k.0 == tenant_id && k.1 == service_id) - { - return Err(DBError::UnknownService { service_id }); - } - - s.service_probes.insert( - (tenant_id, service_probe_id), - ( - ServiceProbeDescr { - service_probe_id, - status: ServiceProbeStatus::Pending, - request: request.clone(), - probe_type: request.probe_type().clone(), - response: None, - // Convert to timestamp to second precision - created_at: convert_bigint_to_time("", created_at.clone().timestamp()).unwrap(), - started_at: None, - finished_at: None, - }, - service_id, - ), - ); - Ok(service_probe_id) - } - - async fn update_service_probe_set_running( - &self, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - started_at: &DateTime, - ) -> Result<(), DBError> { - let mut s = self.lock().await; - - // Service probe must exist - if s.service_probes - .get(&(tenant_id, service_probe_id)) - .is_none() - { - return Err(DBError::UnknownServiceProbe { service_probe_id }.into()); - } - - // Update the service probe - let c = s - .service_probes - .get_mut(&(tenant_id, service_probe_id)) - .ok_or(DBError::UnknownServiceProbe { service_probe_id })?; - c.0.status = ServiceProbeStatus::Running; - c.0.started_at = Some(convert_bigint_to_time("", started_at.clone().timestamp()).unwrap()); - - Ok(()) - } - - async fn update_service_probe_set_finished( - &self, - tenant_id: TenantId, - service_probe_id: ServiceProbeId, - response: ServiceProbeResponse, - finished_at: &DateTime, - ) -> Result<(), DBError> { - let mut s = self.lock().await; - - // Service probe must exist - if s.service_probes - .get(&(tenant_id, service_probe_id)) - .is_none() - { - return Err(DBError::UnknownServiceProbe { service_probe_id }.into()); - } - - // Update the service probe - let c = s - .service_probes - .get_mut(&(tenant_id, service_probe_id)) - .ok_or(DBError::UnknownServiceProbe { service_probe_id })?; - c.0.status = match response { - ServiceProbeResponse::Success(_) => ServiceProbeStatus::Success, - ServiceProbeResponse::Error(_) => ServiceProbeStatus::Failure, - }; - c.0.response = Some(response); - c.0.finished_at = - Some(convert_bigint_to_time("", finished_at.clone().timestamp()).unwrap()); - - Ok(()) - } - - async fn next_service_probe( - &self, - ) -> Result, DBError> - { - let s = self.lock().await; - - // Sort ascending on (timestamp, id) - // Sorting by created_at is at second granularity - let mut values: Vec<(&(TenantId, ServiceProbeId), &(ServiceProbeDescr, ServiceId))> = - Vec::from_iter(s.service_probes.iter()); - values.sort_by(|(_, (descr1, _)), (_, (descr2, _))| { - (descr1.created_at, descr1.service_probe_id) - .cmp(&(descr2.created_at, descr2.service_probe_id)) - }); - - values - .iter() - .find(|(_, (probe_descr, _))| { - probe_descr.status == ServiceProbeStatus::Pending - || probe_descr.status == ServiceProbeStatus::Running - }) - .map( - |((tenant_id, service_probe_id), (probe_descr, service_id))| { - Ok(Some(( - *service_probe_id, - *tenant_id, - probe_descr.request.clone(), - s.services - .get(&(*tenant_id, *service_id)) - .cloned() - .ok_or(DBError::UnknownService { - service_id: *service_id, - })? - .config, - ))) - }, - ) - .unwrap_or(Ok(None)) - } - - async fn get_service_probe( - &self, - tenant_id: TenantId, - service_id: ServiceId, - service_probe_id: ServiceProbeId, - _txn: Option<&Transaction<'_>>, - ) -> Result { - let s = self.lock().await; - - // Service probe must exist - let service_probe = s - .service_probes - .get(&(tenant_id, service_probe_id)) - .cloned() - .ok_or(DBError::UnknownServiceProbe { service_probe_id })?; - - // The service identifier must match to that of the probe - if service_probe.1 != service_id { - return Err(DBError::UnknownServiceProbe { service_probe_id }); - } - - Ok(service_probe.0) - } - - async fn list_service_probes( - &self, - tenant_id: TenantId, - service_id: ServiceId, - limit: Option, - probe_type: Option, - _txn: Option<&Transaction<'_>>, - ) -> Result, DBError> { - let s = self.lock().await; - - // A service with that name must exist, else return empty - let service = s - .services - .iter() - .filter(|k| k.0 .0 == tenant_id) - .map(|k| k.1.clone()) - .find(|c| c.service_id == service_id); - if service.is_none() { - return Ok(vec![]); - } - let service = service.unwrap(); - - // Retrieve all probes - let mut list: Vec = s - .service_probes - .iter() - .filter(|k| k.0 .0 == tenant_id && k.1 .1 == service.service_id) - .map(|(_, (descr, _))| descr.clone()) - .collect(); - - // Sort descending on (timestamp, id) - list.sort_by(|descr1, descr2| { - (descr2.created_at, descr2.service_probe_id) - .cmp(&(descr1.created_at, descr1.service_probe_id)) - }); - - // Filter out leaving a single request type - if let Some(probe_type) = probe_type { - list.retain(|descr| descr.probe_type == probe_type); - } - - // Limit list size - if let Some(limit) = limit { - while list.len() > limit as usize { - list.pop(); - } - } - - Ok(list) - } } diff --git a/crates/pipeline_manager/src/integration_test.rs b/crates/pipeline_manager/src/integration_test.rs index 8e0a783c4c..aa331dea50 100644 --- a/crates/pipeline_manager/src/integration_test.rs +++ b/crates/pipeline_manager/src/integration_test.rs @@ -262,7 +262,6 @@ impl TestConfig { let req = config.delete(format!("/v0/services/{}", name)).await; assert_eq!(StatusCode::OK, req.status(), "Response {:?}", req) } - // .. service probes are deleted through cascading } async fn get>(&self, endpoint: S) -> ClientResponse> { @@ -2260,111 +2259,6 @@ async fn service_basic() { assert_eq!(response.as_array().unwrap().len(), 0); } -/// Wait for the service probe to have a certain status with a one minute timeout. -async fn wait_for_service_probe_status(config: &TestConfig, probe_id: &str, expected_status: &str) { - let start = Instant::now(); - loop { - let mut response = config - .get(format!("/v0/services/example-kafka/probes?id={probe_id}")) - .await; - let value: Value = response.json().await.unwrap(); - let status = value.as_array().unwrap()[0]["status"].as_str().unwrap(); - if status == expected_status { - break; - } - sleep(Duration::from_millis(100)).await; - if Instant::now() - start > Duration::from_secs(60) { - panic!("Waiting for service probe status {expected_status} took too long") - } - } -} - -#[actix_web::test] -#[serial] -async fn service_probe_basic() { - let config = setup().await; - - // Create a service - let request = config - .post( - format!("/v0/services"), - &json!({ - "name": "example-kafka", - "description": "Description of the service used to test service probing", - "config": { - "kafka": { - "bootstrap_servers": [ - "localhost:nonexistent", - ], - "options": {} - } - } - }), - ) - .await; - assert_eq!(request.status(), StatusCode::CREATED); - - // Create a probe for connectivity - let post_body = json!("test_connectivity"); - let mut request = config - .post(format!("/v0/services/example-kafka/probes"), &post_body) - .await; - assert_eq!(request.status(), StatusCode::CREATED); - let response: Value = request.json().await.unwrap(); - let probe_id = response["service_probe_id"].as_str().unwrap(); - wait_for_service_probe_status(&config, probe_id, "failure").await; - - // Create a probe for Kafka topics - let post_body = json!("kafka_get_topics"); - let mut request = config - .post(format!("/v0/services/example-kafka/probes"), &post_body) - .await; - assert_eq!(request.status(), StatusCode::CREATED); - let response: Value = request.json().await.unwrap(); - let probe_id = response["service_probe_id"].as_str().unwrap(); - wait_for_service_probe_status(&config, probe_id, "failure").await; - - // List: no filter - let mut request = config.get_ok("/v0/services/example-kafka/probes").await; - let response: Value = request.json().await.unwrap(); - assert_eq!(response.as_array().unwrap().len(), 2); - - // List: limit 1 - let mut request = config - .get_ok("/v0/services/example-kafka/probes?limit=1") - .await; - let response: Value = request.json().await.unwrap(); - assert_eq!(response.as_array().unwrap().len(), 1); - - // List: limit 0 - let mut request = config - .get_ok("/v0/services/example-kafka/probes?limit=0") - .await; - let response: Value = request.json().await.unwrap(); - assert_eq!(response.as_array().unwrap().len(), 0); - - // List: only test_connectivity - let mut request = config - .get_ok("/v0/services/example-kafka/probes?type=test_connectivity") - .await; - let response: Value = request.json().await.unwrap(); - assert_eq!(response.as_array().unwrap().len(), 1); - - // List: only kafka_get_topics - let mut request = config - .get_ok("/v0/services/example-kafka/probes?type=kafka_get_topics") - .await; - let response: Value = request.json().await.unwrap(); - assert_eq!(response.as_array().unwrap().len(), 1); - - // List: filter limit and request type - let mut request = config - .get_ok("/v0/services/example-kafka/probes?type=kafka_get_topics&limit=0") - .await; - let response: Value = request.json().await.unwrap(); - assert_eq!(response.as_array().unwrap().len(), 0); -} - #[actix_web::test] #[serial] async fn service_integrated_connector_transport() { diff --git a/crates/pipeline_manager/src/lib.rs b/crates/pipeline_manager/src/lib.rs index 3713afaaff..4cffdbaaa0 100644 --- a/crates/pipeline_manager/src/lib.rs +++ b/crates/pipeline_manager/src/lib.rs @@ -15,6 +15,5 @@ pub mod logging; pub mod metrics; pub mod pipeline_automata; pub mod probe; -pub mod prober; pub mod retries; pub mod runner; diff --git a/crates/pipeline_manager/src/prober/mod.rs b/crates/pipeline_manager/src/prober/mod.rs deleted file mode 100644 index 668801db04..0000000000 --- a/crates/pipeline_manager/src/prober/mod.rs +++ /dev/null @@ -1,124 +0,0 @@ -// Exported -pub mod service; - -use crate::api::ManagerError; -use crate::config::ProberConfig; -use crate::db::storage::Storage; -use crate::db::{DBError, ProjectDB}; -use crate::prober::service::probe_service; -use actix_web::{get, web, HttpServer, Responder}; -use log::{debug, error, info}; -use std::sync::Arc; -use std::time::Duration; -use tokio::sync::Mutex; -use tokio::time::sleep; -use tokio::{join, spawn}; - -/// Constantly checks for new probes being created in the database. -/// It performs the probes sequentially one after the other (sequentially). -/// If there are no probes to perform, it briefly sleeps before checking again. -/// -/// Probes which are in `created` or `running` status are fetched one after the -/// other ordered ascending by their `created_at` timestamp (thus, earliest -/// created first). It also fetches `running` status, as currently there is only -/// a single prober service, and it allows recovering if the prober service was -/// unexpectedly terminated in the middle of a probe. -/// -/// TODO: concurrent probing with a sliding window rather than sequential -async fn service_probing_task( - config: ProberConfig, - db: Arc>, -) -> Result<(), ManagerError> { - info!( - "Probing service started (sleeps {} ms before checking again if there are no probes)", - config.probe_sleep_inbetween_ms - ); - loop { - let result: Result<(), DBError> = { - loop { - let probe = db.lock().await.next_service_probe().await?; - if probe.is_none() { - break; - } - let (service_probe_id, tenant_id, request, service_config) = probe.unwrap(); - info!("Probe: {}", service_probe_id); - - // Store in database the probe is started - let started_at = chrono::offset::Utc::now(); - db.lock() - .await - .update_service_probe_set_running(tenant_id, service_probe_id, &started_at) - .await?; - - // Perform the probe with timeout - debug!("Probe request: {:?}", &request); - let response = probe_service( - &service_config, - request, - Duration::from_millis(config.probe_timeout_ms), - ); - let finished_at = chrono::offset::Utc::now(); - debug!("Probe response: {:?}", &response); - - // Store in database the probe is finished - db.lock() - .await - .update_service_probe_set_finished( - tenant_id, - service_probe_id, - response, - &finished_at, - ) - .await?; - } - Ok(()) - }; - match result { - Ok(_) => {} - Err(e) => { - error!("Probing failed unexpectedly: {e}"); - } - }; - - // Wait inbetween probing - sleep(Duration::from_millis(config.probe_sleep_inbetween_ms)).await; - } -} - -/// Health check endpoint. -/// Checks whether the prober is able to reach the database. -#[get("/healthz")] -async fn healthz( - probe: web::Data>>, -) -> Result { - probe.lock().await.status_as_http_response() -} - -/// Runs the prober service, which checks for probes being -/// created in the database. -/// -/// Upon start, two threads are spawned, one which performs the -/// probing and the other an HTTP server to expose service health -/// and statistics. -/// -/// Returns when the two threads are finished and joined. -pub async fn run_prober( - config: &ProberConfig, - db: Arc>, -) -> Result<(), ManagerError> { - let probing_task = spawn(service_probing_task(config.clone(), db.clone())); - let db_probe = web::Data::new(crate::probe::Probe::new(db.clone()).await); - let port = config.prober_http_server_port; - let http = spawn( - HttpServer::new(move || { - actix_web::App::new() - .app_data(db_probe.clone()) - .service(healthz) - }) - .bind(("0.0.0.0", port)) - .unwrap() - .run(), - ); - join!(probing_task, http).0.unwrap()?; - Ok(()) -} diff --git a/crates/pipeline_manager/src/prober/service/kafka.rs b/crates/pipeline_manager/src/prober/service/kafka.rs deleted file mode 100644 index 4b2ebc1e20..0000000000 --- a/crates/pipeline_manager/src/prober/service/kafka.rs +++ /dev/null @@ -1,96 +0,0 @@ -use crate::prober::service::{ - ServiceProbeError, ServiceProbeRequest, ServiceProbeResponse, ServiceProbeResult, -}; -use pipeline_types::service::KafkaService; -use rdkafka::consumer::{BaseConsumer, Consumer}; -use rdkafka::metadata::Metadata; -use rdkafka::ClientConfig; -use std::time::Duration; - -/// Create Kafka client configuration using the service configuration. -fn create_client_config(kafka_service: &KafkaService) -> Result { - let mut client_config = ClientConfig::new(); - let final_options = kafka_service - .generate_final_options() - .map_err(|e| ServiceProbeError::Other(format!("{}", e)))?; - for (k, v) in &final_options { - client_config.set(k.clone(), v.clone()); - } - Ok(client_config) -} - -/// Fetch the metadata using the Kafka client created from the service configuration. -fn fetch_metadata( - kafka_service: &KafkaService, - timeout: Duration, -) -> Result { - let client_config = create_client_config(kafka_service)?; - match client_config.create::() { - Ok(consumer) => match consumer.fetch_metadata(None, timeout) { - Ok(metadata) => Ok(metadata), - Err(e) => Err(ServiceProbeError::Other(e.to_string())), - }, - Err(e) => Err(ServiceProbeError::Other(e.to_string())), - } -} - -/// Perform a probe for the Kafka service. -// TODO: use a context to fetch more detailed log messages -// which are asynchronously reported -pub fn probe_kafka_service( - kafka_service: &KafkaService, - probe: ServiceProbeRequest, - timeout: Duration, -) -> ServiceProbeResponse { - // Only general and Kafka probe requests are valid. - // When new services are added, a match will be done - // with UnsupportedRequest error being returned for others. - match probe { - ServiceProbeRequest::TestConnectivity => match fetch_metadata(kafka_service, timeout) { - Ok(_metadata) => ServiceProbeResponse::Success(ServiceProbeResult::Connected), - Err(e) => ServiceProbeResponse::Error(e), - }, - ServiceProbeRequest::KafkaGetTopics => match fetch_metadata(kafka_service, timeout) { - Ok(metadata) => ServiceProbeResponse::Success(ServiceProbeResult::KafkaTopics( - metadata - .topics() - .iter() - .map(|topic| topic.name().to_string()) - .collect::>(), - )), - Err(e) => ServiceProbeResponse::Error(e), - }, - } -} - -#[cfg(test)] -mod tests { - use crate::prober::service::kafka::create_client_config; - use crate::prober::service::ServiceProbeError; - use pipeline_types::service::KafkaService; - use std::collections::BTreeMap; - - #[test] - fn test_create_client_config() { - // Successful - let kafka_service = KafkaService { - bootstrap_servers: vec!["a".to_string(), "b".to_string()], - options: BTreeMap::from([("key".to_string(), "value".to_string())]), - }; - let client_config = create_client_config(&kafka_service).unwrap(); - assert_eq!(client_config.get("bootstrap.servers").unwrap(), "a,b"); - assert_eq!(client_config.get("key").unwrap(), "value"); - - // Failure: bootstrap.servers is specified in options - let kafka_service = KafkaService { - bootstrap_servers: vec!["a".to_string(), "b".to_string()], - options: BTreeMap::from([("bootstrap.servers".to_string(), "value".to_string())]), - }; - assert_eq!( - create_client_config(&kafka_service).unwrap_err(), - ServiceProbeError::Other( - "bootstrap.servers cannot be set in options as it is a separate field".to_string() - ) - ) - } -} diff --git a/crates/pipeline_manager/src/prober/service/mod.rs b/crates/pipeline_manager/src/prober/service/mod.rs deleted file mode 100644 index 92baef9054..0000000000 --- a/crates/pipeline_manager/src/prober/service/mod.rs +++ /dev/null @@ -1,24 +0,0 @@ -mod kafka; -mod probe; - -use kafka::probe_kafka_service; -use pipeline_types::service::ServiceConfig; -use std::time::Duration; - -// Exported -pub use probe::{ - ServiceProbeError, ServiceProbeRequest, ServiceProbeResponse, ServiceProbeResult, - ServiceProbeStatus, ServiceProbeType, -}; - -/// Perform the probe for the service. -/// Returns with a failure if the timeout is exceeded. -pub fn probe_service( - service_config: &ServiceConfig, - probe: ServiceProbeRequest, - timeout: Duration, -) -> ServiceProbeResponse { - match service_config { - ServiceConfig::Kafka(kafka_service) => probe_kafka_service(kafka_service, probe, timeout), - } -} diff --git a/crates/pipeline_manager/src/prober/service/probe.rs b/crates/pipeline_manager/src/prober/service/probe.rs deleted file mode 100644 index 52f3b8c38c..0000000000 --- a/crates/pipeline_manager/src/prober/service/probe.rs +++ /dev/null @@ -1,291 +0,0 @@ -use crate::db::DBError; -use serde::{Deserialize, Serialize}; -use thiserror::Error as ThisError; -use utoipa::ToSchema; - -/// Service probe status. -/// -/// State transition diagram: -/// ```text -/// Pending -/// │ -/// │ (Prober server picks up the probe) -/// │ -/// ▼ -/// ⌛Running ───► Failure -/// │ -/// ▼ -/// Success -/// ``` -#[derive(Deserialize, Serialize, ToSchema, Eq, PartialEq, Debug, Clone, Copy)] -#[serde(rename_all = "snake_case")] -pub enum ServiceProbeStatus { - /// Probe has not been started. - Pending, - /// Probe has been picked up by the probe server and is running. - Running, - /// Probe has finished running and it succeeded. - Success, - /// Probe has finished running and it failed. - Failure, -} - -impl TryFrom for ServiceProbeStatus { - type Error = DBError; - fn try_from(value: String) -> Result { - match value.as_str() { - "pending" => Ok(Self::Pending), - "running" => Ok(Self::Running), - "success" => Ok(Self::Success), - "failure" => Ok(Self::Failure), - _ => Err(DBError::unknown_service_probe_status(value)), - } - } -} - -impl From for &'static str { - fn from(val: ServiceProbeStatus) -> Self { - match val { - ServiceProbeStatus::Pending => "pending", - ServiceProbeStatus::Running => "running", - ServiceProbeStatus::Success => "success", - ServiceProbeStatus::Failure => "failure", - } - } -} - -/// Enumeration of all possible service probe types. -/// Each type maps to exactly one request variant. -#[derive(Debug, Serialize, Deserialize, ToSchema, Clone, Eq, PartialEq)] -#[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[serde(rename_all = "snake_case")] -pub enum ServiceProbeType { - TestConnectivity, - KafkaGetTopics, -} - -impl TryFrom for ServiceProbeType { - type Error = DBError; - fn try_from(value: String) -> Result { - match value.as_str() { - "test_connectivity" => Ok(Self::TestConnectivity), - "kafka_get_topics" => Ok(Self::KafkaGetTopics), - _ => Err(DBError::unknown_service_probe_type(value)), - } - } -} - -impl From for &'static str { - fn from(val: ServiceProbeType) -> Self { - match val { - ServiceProbeType::TestConnectivity => "test_connectivity", - ServiceProbeType::KafkaGetTopics => "kafka_get_topics", - } - } -} - -/// Enumeration of all possible service probe requests. -#[derive(Debug, Serialize, Deserialize, ToSchema, Clone, Eq, PartialEq)] -#[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[serde(rename_all = "snake_case")] -pub enum ServiceProbeRequest { - /// Tests connectivity to the provided service. For example, for a Kafka service, - /// this probe will check whether the supplied bootstrap configuration and - /// credentials are valid by trying to fetch metadata for all topics. - TestConnectivity, - /// Retrieves the names of all Kafka topics present. - KafkaGetTopics, -} - -impl ServiceProbeRequest { - /// Unique service request type used for classification. - pub fn probe_type(&self) -> ServiceProbeType { - match self { - ServiceProbeRequest::TestConnectivity => ServiceProbeType::TestConnectivity, - ServiceProbeRequest::KafkaGetTopics => ServiceProbeType::KafkaGetTopics, - } - } - - /// Deserialize from provided YAML. - pub fn from_yaml_str(s: &str) -> Self { - serde_yaml::from_str(s).unwrap() - } - - /// Serialize to YAML for storage. - pub fn to_yaml(&self) -> String { - serde_yaml::to_string(&self).unwrap() - } -} - -/// Enumeration of all possible service probe success responses. -#[derive(Debug, Serialize, Deserialize, ToSchema, Clone, Eq, PartialEq)] -#[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[serde(rename_all = "snake_case")] -pub enum ServiceProbeResult { - /// A connection to the service was established. - Connected, - /// The names of all Kafka topics of the service. - KafkaTopics(Vec), -} - -/// Range of possible errors that can occur during a service probe. -/// These are shared across all services. -#[derive(ThisError, Debug, Serialize, Deserialize, ToSchema, Clone, Eq, PartialEq)] -#[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[serde(rename_all = "snake_case")] -pub enum ServiceProbeError { - #[error("Timeout was exceeded")] - TimeoutExceeded, - #[error("Service type ({service_type}) does not support probe type ({probe_type:?})")] - UnsupportedRequest { - service_type: String, - probe_type: String, - }, - #[error("{0}")] - Other(String), -} - -/// Response being either success or error. -#[derive(Debug, Serialize, Deserialize, ToSchema, Clone, Eq, PartialEq)] -#[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[serde(rename_all = "snake_case")] -pub enum ServiceProbeResponse { - // Serialization of the enums are done using with::singleton_map - // because serde_yaml does not support nested enums currently. - // See also: - #[serde(with = "serde_yaml::with::singleton_map")] - Success(ServiceProbeResult), - #[serde(with = "serde_yaml::with::singleton_map")] - Error(ServiceProbeError), -} - -impl ServiceProbeResponse { - /// Deserialize from provided YAML. - pub fn from_yaml_str(s: &str) -> Self { - serde_yaml::from_str(s).unwrap() - } - - /// Serialize to YAML for storage. - pub fn to_yaml(&self) -> String { - serde_yaml::to_string(&self).unwrap() - } -} - -#[cfg(test)] -mod tests { - use super::ServiceProbeType; - use super::{ServiceProbeRequest, ServiceProbeStatus}; - use crate::db::DBError; - - /// Tests that the status string conversion is the same in both directions. - #[test] - fn test_status_string_conversion() { - for (string_repr, corresponding_status) in [ - ("pending", ServiceProbeStatus::Pending), - ("running", ServiceProbeStatus::Running), - ("success", ServiceProbeStatus::Success), - ("failure", ServiceProbeStatus::Failure), - ] { - // Converting status into string (e.g., for storing in database) - assert_eq!( - >::into(corresponding_status.clone()), - string_repr - ); - - // Converting string into status (e.g., for reading from database) - assert_eq!( - ServiceProbeStatus::try_from(string_repr.to_string()).unwrap(), - corresponding_status.clone() - ); - } - - // Converting invalid string into a status fails - match ServiceProbeStatus::try_from("does_not_exist".to_string()) { - Ok(_) => { - panic!("Status converted which does not exist") - } - Err(e) => { - match e { - DBError::UnknownServiceProbeStatus { status, .. } => { - assert_eq!(status, "does_not_exist".to_string()); - } - _ => { - panic!("Incorrect DBError was thrown when converting status that does not exist") - } - } - } - } - } - - /// Tests that the type string conversion and (de-)serialization into JSON is the same. - #[test] - fn test_probe_type_string_conversion_and_de_serialization() { - for (string_repr, corresponding_type) in [ - ("test_connectivity", ServiceProbeType::TestConnectivity), - ("kafka_get_topics", ServiceProbeType::KafkaGetTopics), - ] { - // Converting type into string (e.g., for storing in database) - assert_eq!( - >::into(corresponding_type.clone()), - string_repr - ); - - // Converting string into type (e.g., for reading from database) - assert_eq!( - ServiceProbeType::try_from(string_repr.to_string()).unwrap(), - corresponding_type.clone() - ); - - // Serializing type results in a single JSON string (e.g., in the API output) - assert_eq!( - serde_json::to_string(&corresponding_type).unwrap(), - serde_json::Value::String(string_repr.to_string()).to_string() - ); - assert_eq!( - serde_json::to_string(&corresponding_type).unwrap(), - format!("\"{string_repr}\"") - ); - - // Deserializing a single JSON string results in the type (e.g., from the API input) - assert_eq!( - serde_json::from_str::(format!("\"{string_repr}\"").as_str()) - .unwrap(), - corresponding_type - ); - } - - // Converting invalid string into a type fails - match ServiceProbeType::try_from("does_not_exist".to_string()) { - Ok(_) => { - panic!("Probe type converted which does not exist") - } - Err(e) => { - match e { - DBError::UnknownServiceProbeType { probe_type, .. } => { - assert_eq!(probe_type, "does_not_exist".to_string()); - } - _ => { - panic!("Incorrect DBError was thrown when converting status that does not exist") - } - } - } - } - } - - #[test] - fn test_request_probe_type_and_de_serialization() { - let request = ServiceProbeRequest::TestConnectivity; - assert_eq!(request.probe_type(), ServiceProbeType::TestConnectivity); - assert_eq!( - request, - ServiceProbeRequest::from_yaml_str(&request.to_yaml()) - ); - - let request = ServiceProbeRequest::KafkaGetTopics; - assert_eq!(request.probe_type(), ServiceProbeType::KafkaGetTopics); - assert_eq!( - request, - ServiceProbeRequest::from_yaml_str(&request.to_yaml()) - ); - } -} diff --git a/demo/service-probe/run.py b/demo/service-probe/run.py deleted file mode 100644 index 33dfbb8a84..0000000000 --- a/demo/service-probe/run.py +++ /dev/null @@ -1,73 +0,0 @@ -import time -import requests -import argparse - - -def main(): - # Command-line arguments - parser = argparse.ArgumentParser() - parser.add_argument("--api-url", required=True, help="Feldera API URL (e.g., http://localhost:8080 )") - api_url = parser.parse_args().api_url - - # Kafka server to create service for to probe - pipeline_to_redpanda_server = "redpanda:9092" - - # Create a service - print("Creating service example...") - requests.put(f"{api_url}/v0/services/example", json={ - "description": "Example service", - "config": { - "kafka": { - "bootstrap_servers": [pipeline_to_redpanda_server], - "options": {} - } - } - }).raise_for_status() - - # Probe connectivity - print("Probing connectivity...") - response = requests.post(f"{api_url}/v0/services/example/probes", json="test_connectivity") - response.raise_for_status() - probe_id = response.json()["service_probe_id"] - while True: - probe = requests.get(f"{api_url}/v0/services/example/probes?id={probe_id}").json()[0] - print(f"Status: {probe['status']}") - if probe['status'] in ["failure", "success"]: - print(f"Connectivity test response: {probe['response']}") - break - time.sleep(1) - - # Probe topics - print("Probing topics...") - response = requests.post(f"{api_url}/v0/services/example/probes", json="kafka_get_topics") - response.raise_for_status() - probe_id = response.json()["service_probe_id"] - while True: - probe = requests.get(f"{api_url}/v0/services/example/probes?id={probe_id}").json()[0] - print(f"Status: {probe['status']}") - if probe['status'] in ["failure", "success"]: - print(f"Get topics response: {probe['response']}") - break - time.sleep(1) - - # Retrieve all probes - print("Fetching all probes...") - probes = requests.get(f"{api_url}/v0/services/example/probes").json() - num_pending = sum(map(lambda p: 1 if p['status'] == 'pending' else 0, probes)) - num_running = sum(map(lambda p: 1 if p['status'] == 'running' else 0, probes)) - num_success = sum(map(lambda p: 1 if p['status'] == 'success' else 0, probes)) - num_failure = sum(map(lambda p: 1 if p['status'] == 'failure' else 0, probes)) - print(f"There are in total {len(probes)} probes:") - print(f" > Status pending: {num_pending}") - print(f" > Status running: {num_running}") - print(f" > Status success: {num_success}") - print(f" > Status failure: {num_failure}") - - # Retrieve all probes - print("Retrieving filtered list of probes...") - print("Latest test_connectivity probe: %s" % requests.get(f"{api_url}/v0/services/example/probes?limit=1&type=test_connectivity").json()) - print("Latest kafka_get_topics probe: %s" % requests.get(f"{api_url}/v0/services/example/probes?limit=1&type=kafka_get_topics").json()) - - -if __name__ == "__main__": - main() diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 0013087082..e205b817de 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -184,4 +184,4 @@ services: command: - bash - -c - - "sleep 1 && cd demo/service-probe && python3 run.py --api-url http://pipeline-manager:8080 && sleep 1 && cd ../simple-count && python3 run.py --api-url http://pipeline-manager:8080" + - "sleep 1 && cd demo/simple-count && python3 run.py --api-url http://pipeline-manager:8080" diff --git a/openapi.json b/openapi.json index 7e8b2794bd..3d6b74239f 100644 --- a/openapi.json +++ b/openapi.json @@ -2889,162 +2889,6 @@ } ] } - }, - "/v0/services/{service_name}/probes": { - "get": { - "tags": [ - "Services" - ], - "summary": "Fetch a list of probes for a service, optionally filtered by id.", - "operationId": "list_service_probes", - "parameters": [ - { - "name": "id", - "in": "query", - "description": "If provided, will filter based on exact match of the service probe\nidentifier.", - "required": false, - "schema": { - "type": "string", - "format": "uuid", - "nullable": true - } - }, - { - "name": "limit", - "in": "query", - "description": "If provided, will limit the amount of probes to the N most recent.", - "required": false, - "schema": { - "type": "integer", - "format": "int32", - "nullable": true, - "minimum": 0 - } - }, - { - "name": "type", - "in": "query", - "description": "If provided, will only have probes of that particular type.", - "required": false, - "schema": { - "allOf": [ - { - "$ref": "#/components/schemas/ServiceProbeType" - } - ], - "nullable": true - } - }, - { - "name": "service_name", - "in": "path", - "description": "Unique service name", - "required": true, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "description": "Service probes retrieved successfully.", - "content": { - "application/json": { - "schema": { - "type": "array", - "items": { - "$ref": "#/components/schemas/ServiceProbeDescr" - } - } - } - } - }, - "404": { - "description": "Specified service name does not exist", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorResponse" - }, - "example": { - "details": { - "name": "unknown_name" - }, - "error_code": "UnknownName", - "message": "An entity with name unknown_name was not found" - } - } - } - } - }, - "security": [ - { - "JSON web token (JWT) or API key": [] - } - ] - }, - "post": { - "tags": [ - "Services" - ], - "summary": "Create a service probe.", - "operationId": "new_service_probe", - "parameters": [ - { - "name": "service_name", - "in": "path", - "description": "Unique service name", - "required": true, - "schema": { - "type": "string" - } - } - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ServiceProbeRequest" - } - } - }, - "required": true - }, - "responses": { - "201": { - "description": "Service probe created successfully", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/CreateServiceProbeResponse" - } - } - } - }, - "404": { - "description": "Specified service name does not exist", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorResponse" - }, - "example": { - "details": { - "name": "unknown_name" - }, - "error_code": "UnknownName", - "message": "An entity with name unknown_name was not found" - } - } - } - } - }, - "security": [ - { - "JSON web token (JWT) or API key": [] - } - ] - } } }, "components": { @@ -3539,18 +3383,6 @@ } } }, - "CreateServiceProbeResponse": { - "type": "object", - "description": "Response to a create service probe request.", - "required": [ - "service_probe_id" - ], - "properties": { - "service_probe_id": { - "$ref": "#/components/schemas/ServiceProbeId" - } - } - }, "CsvEncoderConfig": { "type": "object", "properties": { @@ -5042,7 +4874,7 @@ } } ], - "description": "Configuration for a Service, which typically includes how to establish a\nconnection (e.g., hostname, port) and authenticate (e.g., credentials).\n\nThis configuration can be used to easily derive connectors for the service\nas well as probe it for information." + "description": "Configuration for a Service, which typically includes how to establish a\nconnection (e.g., hostname, port) and authenticate (e.g., credentials).\n\nThis configuration can be used to easily derive connectors for the service." }, "ServiceDescr": { "type": "object", @@ -5077,183 +4909,6 @@ "format": "uuid", "description": "Unique service id." }, - "ServiceProbeDescr": { - "type": "object", - "description": "Service probe descriptor.", - "required": [ - "service_probe_id", - "status", - "probe_type", - "request", - "created_at" - ], - "properties": { - "created_at": { - "type": "string", - "format": "date-time" - }, - "finished_at": { - "type": "string", - "format": "date-time", - "nullable": true - }, - "probe_type": { - "$ref": "#/components/schemas/ServiceProbeType" - }, - "request": { - "$ref": "#/components/schemas/ServiceProbeRequest" - }, - "response": { - "allOf": [ - { - "$ref": "#/components/schemas/ServiceProbeResponse" - } - ], - "nullable": true - }, - "service_probe_id": { - "$ref": "#/components/schemas/ServiceProbeId" - }, - "started_at": { - "type": "string", - "format": "date-time", - "nullable": true - }, - "status": { - "$ref": "#/components/schemas/ServiceProbeStatus" - } - } - }, - "ServiceProbeError": { - "oneOf": [ - { - "type": "string", - "enum": [ - "timeout_exceeded" - ] - }, - { - "type": "object", - "required": [ - "unsupported_request" - ], - "properties": { - "unsupported_request": { - "type": "object", - "required": [ - "service_type", - "probe_type" - ], - "properties": { - "probe_type": { - "type": "string" - }, - "service_type": { - "type": "string" - } - } - } - } - }, - { - "type": "object", - "required": [ - "other" - ], - "properties": { - "other": { - "type": "string" - } - } - } - ], - "description": "Range of possible errors that can occur during a service probe.\nThese are shared across all services." - }, - "ServiceProbeId": { - "type": "string", - "format": "uuid", - "description": "Unique service probe id." - }, - "ServiceProbeRequest": { - "type": "string", - "description": "Enumeration of all possible service probe requests.", - "enum": [ - "test_connectivity", - "kafka_get_topics" - ] - }, - "ServiceProbeResponse": { - "oneOf": [ - { - "type": "object", - "required": [ - "success" - ], - "properties": { - "success": { - "$ref": "#/components/schemas/ServiceProbeResult" - } - } - }, - { - "type": "object", - "required": [ - "error" - ], - "properties": { - "error": { - "$ref": "#/components/schemas/ServiceProbeError" - } - } - } - ], - "description": "Response being either success or error." - }, - "ServiceProbeResult": { - "oneOf": [ - { - "type": "string", - "description": "A connection to the service was established.", - "enum": [ - "connected" - ] - }, - { - "type": "object", - "required": [ - "kafka_topics" - ], - "properties": { - "kafka_topics": { - "type": "array", - "items": { - "type": "string" - }, - "description": "The names of all Kafka topics of the service." - } - } - } - ], - "description": "Enumeration of all possible service probe success responses." - }, - "ServiceProbeStatus": { - "type": "string", - "description": "Service probe status.\n\nState transition diagram:\n```text\nPending\n│\n│ (Prober server picks up the probe)\n│\n▼\n⌛Running ───► Failure\n│\n▼\nSuccess\n```", - "enum": [ - "pending", - "running", - "success", - "failure" - ] - }, - "ServiceProbeType": { - "type": "string", - "description": "Enumeration of all possible service probe types.\nEach type maps to exactly one request variant.", - "enum": [ - "test_connectivity", - "kafka_get_topics" - ] - }, "SqlCompilerMessage": { "type": "object", "description": "A SQL compiler error.\n\nThe SQL compiler returns a list of errors in the following JSON format if\nit's invoked with the `-je` option.\n\n```ignore\n[ {\n\"startLineNumber\" : 14,\n\"startColumn\" : 13,\n\"endLineNumber\" : 14,\n\"endColumn\" : 13,\n\"warning\" : false,\n\"errorType\" : \"Error parsing SQL\",\n\"message\" : \"Encountered \\\"\\\" at line 14, column 13.\"\n} ]\n```", From 190c5750a4fd580066e95daf27e86c441a148e0a Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Fri, 5 Jul 2024 20:09:24 +0200 Subject: [PATCH 10/18] Revert "Switch to tracing for logging." This reverts commit 7a7ebb40b769c3464eb17b4cedd43bf1d2384b1a. Signed-off-by: Gerd Zellweger --- Cargo.lock | 6 ++--- crates/adapters/Cargo.toml | 2 +- crates/adapters/src/controller/mod.rs | 2 +- crates/adapters/src/controller/stats.rs | 2 +- crates/adapters/src/format/avro/output.rs | 4 +-- crates/adapters/src/format/json/input.rs | 2 +- crates/adapters/src/format/json/output.rs | 2 +- crates/adapters/src/format/parquet/test.rs | 2 +- .../src/integrated/delta_table/input.rs | 2 +- .../src/integrated/delta_table/output.rs | 2 +- .../src/integrated/delta_table/test.rs | 2 +- crates/adapters/src/server/error.rs | 26 +++++-------------- crates/adapters/src/server/mod.rs | 6 ++--- crates/adapters/src/test/http.rs | 2 +- crates/adapters/src/test/kafka.rs | 2 +- crates/adapters/src/test/mod.rs | 17 ++++++++++++ crates/adapters/src/transport/http/input.rs | 2 +- crates/adapters/src/transport/http/output.rs | 4 +-- .../adapters/src/transport/kafka/ft/input.rs | 2 +- crates/adapters/src/transport/kafka/ft/mod.rs | 2 +- .../adapters/src/transport/kafka/ft/output.rs | 2 +- .../adapters/src/transport/kafka/ft/test.rs | 2 +- crates/adapters/src/transport/kafka/mod.rs | 12 ++++----- .../src/transport/kafka/nonft/input.rs | 2 +- .../src/transport/kafka/nonft/output.rs | 2 +- .../src/transport/kafka/nonft/test.rs | 2 +- crates/adapters/src/transport/s3/mod.rs | 2 +- .../adapters/src/transport/secret_resolver.rs | 2 +- crates/dbsp/Cargo.toml | 4 +-- crates/dbsp/src/circuit/checkpointer.rs | 12 ++++----- crates/dbsp/src/circuit/runtime.rs | 2 +- crates/dbsp/src/error.rs | 4 +-- crates/dbsp/src/storage/backend/mod.rs | 2 +- crates/dbsp/src/storage/dirlock/mod.rs | 6 ++--- crates/dbsp/src/storage/mod.rs | 2 +- crates/dbsp/src/trace/spine_async/merger.rs | 2 +- 36 files changed, 78 insertions(+), 73 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 531390d4ff..59d6971bed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3717,7 +3717,8 @@ dependencies = [ "itertools 0.10.5", "lazy_static", "libc", - "metrics 0.23.0", + "log", + "metrics 0.22.3", "mimalloc-rust-sys", "nix 0.27.1", "num", @@ -3754,7 +3755,6 @@ dependencies = [ "thiserror", "time", "tokio", - "tracing", "typedmap", "uuid", "xxhash-rust", @@ -3798,6 +3798,7 @@ dependencies = [ "futures-util", "jemalloc_pprof", "lazy_static", + "log", "metrics 0.23.0", "metrics-exporter-prometheus", "metrics-util 0.17.0", @@ -3835,7 +3836,6 @@ dependencies = [ "tempfile", "test_bin", "tokio", - "tracing", "url", "utoipa", "uuid", diff --git a/crates/adapters/Cargo.toml b/crates/adapters/Cargo.toml index 6efabd3017..cb7a077d73 100644 --- a/crates/adapters/Cargo.toml +++ b/crates/adapters/Cargo.toml @@ -44,7 +44,7 @@ aws-types = "1.1.7" actix = "0.13.1" actix-web = { version = "4.4.0", default-features = false, features = ["cookies", "macros", "compress-gzip", "compress-brotli"] } mime = "0.3.16" -tracing = { version = "0.1.40" } +log = "0.4.20" # Once chrono is released with `849932` chrono version needs to be updated in size-of crate: size-of = { git = "https://github.com/gz/size-of.git", rev = "3ec40db", features = ["time-std", "ordered-float"], optional = true } futures = { version = "0.3.28" } diff --git a/crates/adapters/src/controller/mod.rs b/crates/adapters/src/controller/mod.rs index dffff4c1c1..5168b39e72 100644 --- a/crates/adapters/src/controller/mod.rs +++ b/crates/adapters/src/controller/mod.rs @@ -51,6 +51,7 @@ use crossbeam::{ }; use dbsp::circuit::{CircuitConfig, Layout}; use dbsp::profile::GraphProfile; +use log::{debug, error, info, trace}; use metrics::set_global_recorder; use metrics_exporter_prometheus::{PrometheusBuilder, PrometheusHandle}; use metrics_util::{ @@ -73,7 +74,6 @@ use std::{ thread::{spawn, JoinHandle}, time::{Duration, Instant}, }; -use tracing::{debug, error, info, trace}; use uuid::Uuid; mod error; diff --git a/crates/adapters/src/controller/stats.rs b/crates/adapters/src/controller/stats.rs index f5ccc10332..6397b07dfc 100644 --- a/crates/adapters/src/controller/stats.rs +++ b/crates/adapters/src/controller/stats.rs @@ -37,6 +37,7 @@ use crate::{ }; use anyhow::Error as AnyError; use crossbeam::sync::{ShardedLock, ShardedLockReadGuard, Unparker}; +use log::error; use metrics::{KeyName, SharedString as MetricString, Unit as MetricUnit}; use metrics_util::{ debugging::{DebugValue, Snapshot}, @@ -57,7 +58,6 @@ use std::{ Mutex, }, }; -use tracing::error; #[derive(Default, Serialize)] pub struct GlobalControllerMetrics { diff --git a/crates/adapters/src/format/avro/output.rs b/crates/adapters/src/format/avro/output.rs index 15e410dab2..5482792534 100644 --- a/crates/adapters/src/format/avro/output.rs +++ b/crates/adapters/src/format/avro/output.rs @@ -5,6 +5,7 @@ use actix_web::HttpRequest; use anyhow::{anyhow, bail, Result as AnyResult}; use apache_avro::{to_avro_datum, Schema as AvroSchema}; use erased_serde::Serialize as ErasedSerialize; +use log::{debug, error}; use pipeline_types::format::avro::AvroEncoderConfig; use pipeline_types::program_schema::Relation; use schema_registry_converter::avro_common::get_supplied_schema; @@ -15,7 +16,6 @@ use serde_yaml::Value as YamlValue; use std::borrow::Cow; use std::str::FromStr; use std::time::Duration; -use tracing::{debug, error}; // TODOs: // - This connector currently only supports raw Avro format, i.e., deletes cannot be represented. @@ -312,11 +312,11 @@ mod test { use crate::{Encoder, SerBatch}; use dbsp::utils::Tup2; use dbsp::{DBData, OrdZSet}; + use log::trace; use pipeline_types::serde_with_context::{SerializeWithContext, SqlSerdeConfig}; use proptest::proptest; use serde::Deserialize; use std::sync::Arc; - use tracing::trace; fn test_avro(avro_schema: &str, batches: Vec>>) where diff --git a/crates/adapters/src/format/json/input.rs b/crates/adapters/src/format/json/input.rs index f37b17dcdd..ef2742986c 100644 --- a/crates/adapters/src/format/json/input.rs +++ b/crates/adapters/src/format/json/input.rs @@ -452,13 +452,13 @@ mod test { transport::InputConsumer, FormatConfig, ParseError, }; + use log::trace; use pipeline_types::{ deserialize_table_record, format::json::{JsonFlavor, JsonParserConfig, JsonUpdateFormat}, serde_with_context::{DeserializeWithContext, SqlSerdeConfig}, }; use std::{borrow::Cow, fmt::Debug}; - use tracing::trace; #[derive(PartialEq, Debug, Eq)] struct TestStruct { diff --git a/crates/adapters/src/format/json/output.rs b/crates/adapters/src/format/json/output.rs index 63ab2bd0ee..2e7313f86a 100644 --- a/crates/adapters/src/format/json/output.rs +++ b/crates/adapters/src/format/json/output.rs @@ -386,12 +386,12 @@ mod test { test::{generate_test_batches_with_weights, MockOutputConsumer, TestStruct}, }; use dbsp::{utils::Tup2, OrdZSet}; + use log::trace; use pipeline_types::format::json::JsonUpdateFormat; use pipeline_types::program_schema::Relation; use proptest::prelude::*; use serde::Deserialize; use std::{cell::RefCell, fmt::Debug, rc::Rc, sync::Arc}; - use tracing::trace; trait OutputUpdate: Debug + for<'de> Deserialize<'de> + Eq + Ord { type Val; diff --git a/crates/adapters/src/format/parquet/test.rs b/crates/adapters/src/format/parquet/test.rs index b26c9336e8..c0a3c9eaea 100644 --- a/crates/adapters/src/format/parquet/test.rs +++ b/crates/adapters/src/format/parquet/test.rs @@ -173,6 +173,6 @@ fn debug_parquet_buffer(buffer: Vec) { .expect("Row iterator creation should succeed"); for maybe_record in row_iter { let record = maybe_record.expect("Record should be read successfully"); - tracing::info!("record = {:?}", record.to_string()); + log::info!("record = {:?}", record.to_string()); } } diff --git a/crates/adapters/src/integrated/delta_table/input.rs b/crates/adapters/src/integrated/delta_table/input.rs index 3697bdc4b9..a0f00255d5 100644 --- a/crates/adapters/src/integrated/delta_table/input.rs +++ b/crates/adapters/src/integrated/delta_table/input.rs @@ -27,6 +27,7 @@ use deltalake::table::PeekCommit; use deltalake::{datafusion, DeltaTable, DeltaTableBuilder, Path}; use env_logger::builder; use futures_util::StreamExt; +use log::{debug, error, info, trace}; use pipeline_types::config::{InputEndpointConfig, TransportConfigVariant}; use pipeline_types::format::json::JsonFlavor; use pipeline_types::program_schema::Relation; @@ -42,7 +43,6 @@ use tokio::select; use tokio::sync::mpsc; use tokio::sync::watch::{channel, Receiver, Sender}; use tokio::time::sleep; -use tracing::{debug, error, info, trace}; use url::Url; use utoipa::ToSchema; diff --git a/crates/adapters/src/integrated/delta_table/output.rs b/crates/adapters/src/integrated/delta_table/output.rs index f6e7544025..0298ec0bbf 100644 --- a/crates/adapters/src/integrated/delta_table/output.rs +++ b/crates/adapters/src/integrated/delta_table/output.rs @@ -20,6 +20,7 @@ use deltalake::operations::transaction::{CommitBuilder, TableReference}; use deltalake::operations::writer::{DeltaWriter, WriterConfig}; use deltalake::protocol::{DeltaOperation, SaveMode}; use deltalake::DeltaTable; +use log::{debug, error, info, trace}; use pipeline_types::transport::delta_table::DeltaTableWriteMode; use pipeline_types::{program_schema::Relation, transport::delta_table::DeltaTableWriterConfig}; use serde_arrow::schema::SerdeArrowSchema; @@ -27,7 +28,6 @@ use serde_arrow::ArrayBuilder; use std::cmp::min; use std::sync::{Arc, Weak}; use tokio::sync::mpsc::{channel, Receiver, Sender}; -use tracing::{debug, error, info, trace}; struct DeltaTableWriterInner { endpoint_id: EndpointId, diff --git a/crates/adapters/src/integrated/delta_table/test.rs b/crates/adapters/src/integrated/delta_table/test.rs index a810faef40..1cd0d3eee1 100644 --- a/crates/adapters/src/integrated/delta_table/test.rs +++ b/crates/adapters/src/integrated/delta_table/test.rs @@ -25,6 +25,7 @@ use deltalake::operations::create::CreateBuilder; use deltalake::protocol::SaveMode; use deltalake::{DeltaOps, DeltaTable, DeltaTableBuilder}; use env_logger::Env; +use log::{debug, info, trace}; use parquet::file::reader::Length; use pipeline_types::config::PipelineConfig; use pipeline_types::program_schema::{Field, Relation}; @@ -51,7 +52,6 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use tempfile::{tempdir, NamedTempFile, TempDir}; use tokio::time::sleep; -use tracing::{debug, info, trace}; use uuid::Uuid; fn delta_output_serde_config() -> SqlSerdeConfig { diff --git a/crates/adapters/src/server/error.rs b/crates/adapters/src/server/error.rs index ed8cbd9d07..ea604bc11a 100644 --- a/crates/adapters/src/server/error.rs +++ b/crates/adapters/src/server/error.rs @@ -46,6 +46,7 @@ use actix_web::{ }; use anyhow::Error as AnyError; use dbsp::{operator::sample::MAX_QUANTILES, DetailedError}; +use log::{error, log, warn, Level}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value as JsonValue}; use std::{ @@ -54,7 +55,6 @@ use std::{ fmt::{Display, Error as FmtError, Formatter}, sync::Arc, }; -use tracing::{error, warn, Level}; use utoipa::ToSchema; pub const MAX_REPORTED_PARSE_ERRORS: usize = 1_000; @@ -83,18 +83,6 @@ where } } -macro_rules! dyn_event { - ($lvl:ident, $($arg:tt)+) => { - match $lvl { - ::tracing::Level::TRACE => ::tracing::trace!($($arg)+), - ::tracing::Level::DEBUG => ::tracing::debug!($($arg)+), - ::tracing::Level::INFO => ::tracing::info!($($arg)+), - ::tracing::Level::WARN => ::tracing::warn!($($arg)+), - ::tracing::Level::ERROR => ::tracing::error!($($arg)+), - } - }; -} - impl ErrorResponse { pub fn from_anyerror(error: &AnyError) -> Self { let message = error.to_string(); @@ -115,9 +103,9 @@ impl ErrorResponse { E: DetailedError, { let result = Self::from_error_nolog(error); - let level = error.log_level(); - dyn_event!( - level, + + log!( + error.log_level(), "[HTTP error response] {}: {}", result.error_code, result.message @@ -294,10 +282,10 @@ impl DetailedError for PipelineError { fn log_level(&self) -> Level { match self { - Self::Initializing => Level::INFO, - Self::Terminating => Level::INFO, + Self::Initializing => Level::Info, + Self::Terminating => Level::Info, Self::ControllerError { error } => error.log_level(), - _ => Level::ERROR, + _ => Level::Error, } } } diff --git a/crates/adapters/src/server/mod.rs b/crates/adapters/src/server/mod.rs index 3a107e3bd7..1e536acba1 100644 --- a/crates/adapters/src/server/mod.rs +++ b/crates/adapters/src/server/mod.rs @@ -22,6 +22,7 @@ use colored::Colorize; use dbsp::circuit::CircuitConfig; use dbsp::operator::sample::MAX_QUANTILES; use env_logger::Env; +use log::{debug, error, info, warn}; use pipeline_types::{format::json::JsonFlavor, transport::http::EgressMode}; use pipeline_types::{query::OutputQuery, transport::http::SERVER_PORT_FILE}; use serde::Deserialize; @@ -44,7 +45,6 @@ use tokio::{ oneshot, }, }; -use tracing::{debug, error, info, warn}; use uuid::Uuid; pub mod error; @@ -980,8 +980,8 @@ mod test_with_kafka { .unwrap() .current(); - //let _ = tracing::set_logger(&TEST_LOGGER); - //tracing::set_max_level(LevelFilter::Debug); + //let _ = log::set_logger(&TEST_LOGGER); + //log::set_max_level(LevelFilter::Debug); // Create topics. let kafka_resources = KafkaResources::create_topics(&[ diff --git a/crates/adapters/src/test/http.rs b/crates/adapters/src/test/http.rs index 491d3cf99e..75724e466b 100644 --- a/crates/adapters/src/test/http.rs +++ b/crates/adapters/src/test/http.rs @@ -7,7 +7,7 @@ use awc::{error::PayloadError, ClientRequest}; use csv::ReaderBuilder as CsvReaderBuilder; use csv::WriterBuilder as CsvWriterBuilder; use futures::{Stream, StreamExt}; -use tracing::trace; +use log::trace; pub struct TestHttpSender; pub struct TestHttpReceiver; diff --git a/crates/adapters/src/test/kafka.rs b/crates/adapters/src/test/kafka.rs index db9cad2a78..a8bb48938a 100644 --- a/crates/adapters/src/test/kafka.rs +++ b/crates/adapters/src/test/kafka.rs @@ -7,6 +7,7 @@ use anyhow::{anyhow, bail, Result as AnyResult}; use csv::WriterBuilder as CsvWriterBuilder; use futures::executor::block_on; use lazy_static::lazy_static; +use log::{error, info}; use pipeline_types::program_schema::Relation; use pipeline_types::transport::kafka::default_redpanda_server; use rdkafka::message::BorrowedMessage; @@ -28,7 +29,6 @@ use std::{ thread::{sleep, JoinHandle}, time::{Duration, Instant, SystemTime, UNIX_EPOCH}, }; -use tracing::{error, info}; static MAX_TOPIC_PROBE_TIMEOUT: Duration = Duration::from_millis(20_000); diff --git a/crates/adapters/src/test/mod.rs b/crates/adapters/src/test/mod.rs index a2b8902162..7274d6ee60 100644 --- a/crates/adapters/src/test/mod.rs +++ b/crates/adapters/src/test/mod.rs @@ -6,6 +6,7 @@ use crate::{ }; use anyhow::Result as AnyResult; use dbsp::{DBData, OrdZSet, Runtime}; +use log::{Log, Metadata, Record}; use pipeline_types::serde_with_context::{ DeserializeWithContext, SerializeWithContext, SqlSerdeConfig, }; @@ -42,8 +43,24 @@ pub use mock_input_consumer::MockInputConsumer; pub use mock_output_consumer::MockOutputConsumer; use pipeline_types::program_schema::{Field, Relation}; +pub struct TestLogger; + +pub static TEST_LOGGER: TestLogger = TestLogger; + pub static DEFAULT_TIMEOUT_MS: u128 = 600_000; +impl Log for TestLogger { + fn enabled(&self, _metadata: &Metadata) -> bool { + true + } + + fn log(&self, record: &Record) { + println!("{} - {}", record.level(), record.args()); + } + + fn flush(&self) {} +} + /// Wait for `predicate` to become `true`. /// /// Returns the number of milliseconds elapsed or `Err(())` on timeout. diff --git a/crates/adapters/src/transport/http/input.rs b/crates/adapters/src/transport/http/input.rs index 2a3b708b45..b5dc7056a7 100644 --- a/crates/adapters/src/transport/http/input.rs +++ b/crates/adapters/src/transport/http/input.rs @@ -9,6 +9,7 @@ use actix_web::{web::Payload, HttpResponse}; use anyhow::{anyhow, Error as AnyError, Result as AnyResult}; use circular_queue::CircularQueue; use futures_util::StreamExt; +use log::debug; use num_traits::FromPrimitive; use serde::Deserialize; use std::{ @@ -19,7 +20,6 @@ use std::{ time::Duration, }; use tokio::{sync::watch, time::timeout}; -use tracing::debug; #[derive(Clone, Debug, Deserialize)] pub(crate) enum HttpIngressMode { diff --git a/crates/adapters/src/transport/http/output.rs b/crates/adapters/src/transport/http/output.rs index b3f4935673..d46c25f5d5 100644 --- a/crates/adapters/src/transport/http/output.rs +++ b/crates/adapters/src/transport/http/output.rs @@ -3,6 +3,8 @@ use actix_web::{http::header::ContentType, web::Bytes, HttpResponse}; use anyhow::{anyhow, bail, Result as AnyResult}; use async_stream::stream; use crossbeam::sync::ShardedLock; +use log::debug; +use log::error; use serde::{ser::SerializeStruct, Serializer}; use serde_json::value::RawValue; use std::{ @@ -16,8 +18,6 @@ use tokio::{ sync::{mpsc, oneshot}, time::timeout, }; -use tracing::debug; -use tracing::error; // TODO: make this configurable via endpoint config. const MAX_BUFFERS: usize = 100; diff --git a/crates/adapters/src/transport/kafka/ft/input.rs b/crates/adapters/src/transport/kafka/ft/input.rs index 86699e4187..bf29854aac 100644 --- a/crates/adapters/src/transport/kafka/ft/input.rs +++ b/crates/adapters/src/transport/kafka/ft/input.rs @@ -5,6 +5,7 @@ use crate::{InputConsumer, TransportInputEndpoint}; use anyhow::{anyhow, bail, Context, Error as AnyError, Result as AnyResult}; use crossbeam::sync::{Parker, Unparker}; use futures::executor::block_on; +use log::{debug, error, info, warn}; use pipeline_types::transport::kafka::KafkaInputConfig; use rdkafka::admin::{AdminClient, AdminOptions, NewTopic}; use rdkafka::config::{FromClientConfig, FromClientConfigAndContext}; @@ -31,7 +32,6 @@ use std::{ sync::{Arc, Mutex}, thread::{Builder, JoinHandle}, }; -use tracing::{debug, error, info, warn}; use super::CommonConfig; diff --git a/crates/adapters/src/transport/kafka/ft/mod.rs b/crates/adapters/src/transport/kafka/ft/mod.rs index 2429ad7135..475316fcc0 100644 --- a/crates/adapters/src/transport/kafka/ft/mod.rs +++ b/crates/adapters/src/transport/kafka/ft/mod.rs @@ -16,6 +16,7 @@ mod output; use crate::transport::kafka::refine_kafka_error; use anyhow::{anyhow, bail, Context, Error as AnyError, Result as AnyResult}; +use log::{debug, error, info, warn}; use pipeline_types::transport::kafka::{default_redpanda_server, KafkaLogLevel}; use rdkafka::{ client::Client as KafkaClient, @@ -37,7 +38,6 @@ use std::{ sync::Arc, time::Duration, }; -use tracing::{debug, error, info, warn}; use uuid::Uuid; pub use input::Endpoint as KafkaFtInputEndpoint; diff --git a/crates/adapters/src/transport/kafka/ft/output.rs b/crates/adapters/src/transport/kafka/ft/output.rs index 8e0b8520b8..14d778f5e6 100644 --- a/crates/adapters/src/transport/kafka/ft/output.rs +++ b/crates/adapters/src/transport/kafka/ft/output.rs @@ -4,6 +4,7 @@ use crate::{ AsyncErrorCallback, OutputEndpoint, }; use anyhow::{anyhow, bail, Context, Error as AnyError, Result as AnyResult}; +use log::{debug, info, warn}; use pipeline_types::transport::kafka::KafkaOutputConfig; use rdkafka::message::OwnedHeaders; use rdkafka::{ @@ -20,7 +21,6 @@ use std::{ sync::{Condvar, Mutex, RwLock}, time::Duration, }; -use tracing::{debug, info, warn}; use super::{count_partitions_in_topic, CommonConfig, Ctp, DataConsumerContext}; diff --git a/crates/adapters/src/transport/kafka/ft/test.rs b/crates/adapters/src/transport/kafka/ft/test.rs index 08fa3f03a0..595b828e7d 100644 --- a/crates/adapters/src/transport/kafka/ft/test.rs +++ b/crates/adapters/src/transport/kafka/ft/test.rs @@ -11,6 +11,7 @@ use crate::{ use anyhow::Error as AnyError; use crossbeam::sync::{Parker, Unparker}; use env_logger::Env; +use log::info; use proptest::prelude::*; use rdkafka::mocking::MockCluster; use std::{ @@ -22,7 +23,6 @@ use std::{ thread::sleep, time::{Duration, Instant}, }; -use tracing::info; use uuid::Uuid; /// Wait to receive all records in `data` in the same order. diff --git a/crates/adapters/src/transport/kafka/mod.rs b/crates/adapters/src/transport/kafka/mod.rs index ea30e5407c..90dd63bb42 100644 --- a/crates/adapters/src/transport/kafka/mod.rs +++ b/crates/adapters/src/transport/kafka/mod.rs @@ -104,7 +104,7 @@ impl DeferredLogging { *self.0.lock().unwrap() = Some(Vec::new()); let r = f(); for (level, fac, message) in self.0.lock().unwrap().take().unwrap().drain(..) { - tracing::info!("{level:?} {fac} {message}"); + log::info!("{level:?} {fac} {message}"); } r } @@ -151,19 +151,19 @@ impl DeferredLogging { | RDKafkaLogLevel::Alert | RDKafkaLogLevel::Critical | RDKafkaLogLevel::Error => { - tracing::error!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) + log::error!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) } RDKafkaLogLevel::Warning => { - tracing::warn!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) + log::warn!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) } RDKafkaLogLevel::Notice => { - tracing::info!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) + log::info!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) } RDKafkaLogLevel::Info => { - tracing::info!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) + log::info!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) } RDKafkaLogLevel::Debug => { - tracing::debug!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) + log::debug!(target: "librdkafka", "librdkafka: {} {}", fac, log_message) } } } diff --git a/crates/adapters/src/transport/kafka/nonft/input.rs b/crates/adapters/src/transport/kafka/nonft/input.rs index a14082d392..d49f8563a8 100644 --- a/crates/adapters/src/transport/kafka/nonft/input.rs +++ b/crates/adapters/src/transport/kafka/nonft/input.rs @@ -9,6 +9,7 @@ use crate::{ }; use anyhow::{anyhow, bail, Error as AnyError, Result as AnyResult}; use crossbeam::queue::ArrayQueue; +use log::debug; use num_traits::FromPrimitive; use pipeline_types::{secret_ref::MaybeSecretRef, transport::kafka::KafkaInputConfig}; use rdkafka::config::RDKafkaLogLevel; @@ -31,7 +32,6 @@ use std::{ thread::spawn, time::{Duration, Instant}, }; -use tracing::debug; const POLL_TIMEOUT: Duration = Duration::from_millis(100); diff --git a/crates/adapters/src/transport/kafka/nonft/output.rs b/crates/adapters/src/transport/kafka/nonft/output.rs index 291382444d..b839f48ef2 100644 --- a/crates/adapters/src/transport/kafka/nonft/output.rs +++ b/crates/adapters/src/transport/kafka/nonft/output.rs @@ -3,6 +3,7 @@ use crate::transport::secret_resolver::MaybeSecret; use crate::{AsyncErrorCallback, OutputEndpoint}; use anyhow::{anyhow, bail, Error as AnyError, Result as AnyResult}; use crossbeam::sync::{Parker, Unparker}; +use log::debug; use pipeline_types::secret_ref::MaybeSecretRef; use pipeline_types::transport::kafka::KafkaOutputConfig; use rdkafka::message::OwnedHeaders; @@ -14,7 +15,6 @@ use rdkafka::{ ClientConfig, ClientContext, }; use std::{sync::RwLock, time::Duration}; -use tracing::debug; const OUTPUT_POLLING_INTERVAL: Duration = Duration::from_millis(100); diff --git a/crates/adapters/src/transport/kafka/nonft/test.rs b/crates/adapters/src/transport/kafka/nonft/test.rs index 70d1a8236f..99f8e0cc6d 100644 --- a/crates/adapters/src/transport/kafka/nonft/test.rs +++ b/crates/adapters/src/transport/kafka/nonft/test.rs @@ -7,6 +7,7 @@ use crate::{ Controller, PipelineConfig, }; use env_logger::Env; +use log::info; use parquet::data_type::AsBytes; use proptest::prelude::*; use rdkafka::message::{BorrowedMessage, Header, Headers}; @@ -20,7 +21,6 @@ use std::{ thread::sleep, time::Duration, }; -use tracing::info; /// Wait to receive all records in `data` in the same order. fn wait_for_output_ordered(zset: &MockDeZSet, data: &[Vec]) { diff --git a/crates/adapters/src/transport/s3/mod.rs b/crates/adapters/src/transport/s3/mod.rs index be117750b0..b877249af4 100644 --- a/crates/adapters/src/transport/s3/mod.rs +++ b/crates/adapters/src/transport/s3/mod.rs @@ -1,11 +1,11 @@ use std::sync::Arc; use aws_sdk_s3::operation::{get_object::GetObjectOutput, list_objects_v2::ListObjectsV2Error}; +use log::error; use tokio::sync::{ mpsc, watch::{channel, Receiver, Sender}, }; -use tracing::error; use crate::{InputConsumer, InputReader, PipelineState, TransportInputEndpoint}; use dbsp::circuit::tokio::TOKIO; diff --git a/crates/adapters/src/transport/secret_resolver.rs b/crates/adapters/src/transport/secret_resolver.rs index b9eb570681..261ed19efb 100644 --- a/crates/adapters/src/transport/secret_resolver.rs +++ b/crates/adapters/src/transport/secret_resolver.rs @@ -6,9 +6,9 @@ use std::fs; use std::path::Path; use anyhow::{anyhow, Result as AnyResult}; +use log::debug; use pipeline_types::secret_ref::MaybeSecretRef; use regex::Regex; -use tracing::debug; /// Enumeration which holds a simple string or a resolved secret's string. /// diff --git a/crates/dbsp/Cargo.toml b/crates/dbsp/Cargo.toml index edbdd521e0..d2903102a9 100644 --- a/crates/dbsp/Cargo.toml +++ b/crates/dbsp/Cargo.toml @@ -63,8 +63,8 @@ thiserror = "1.0" uuid = { version = "1.6.1", features = ["v7"] } clap = { version = "4.4.14", features = ["derive", "env", "wrap_help"] } fdlimit = { version = "0.3.0" } -metrics = { version = "0.23.0" } -tracing = { version = "0.1.40" } +metrics = { version = "0.22.0" } +log = { version = "0.4", features = [] } rlimit = "0.10.1" serde = { version = "1.0", features = ["derive"] } ptr_meta = "0.2.0" diff --git a/crates/dbsp/src/circuit/checkpointer.rs b/crates/dbsp/src/circuit/checkpointer.rs index ad6e96e94c..52a444311d 100644 --- a/crates/dbsp/src/circuit/checkpointer.rs +++ b/crates/dbsp/src/circuit/checkpointer.rs @@ -126,15 +126,15 @@ impl Checkpointer { ) || path.is_dir() { if path.is_dir() { - tracing::debug!("Removing unused directory '{}'", path.display()); + log::debug!("Removing unused directory '{}'", path.display()); fs::remove_dir_all(path)?; } else { match fs::remove_file(path) { Ok(_) => { - tracing::debug!("Removed unused file '{}'", path.display()); + log::debug!("Removed unused file '{}'", path.display()); } Err(e) => { - tracing::warn!("Unable to remove old-checkpoint file {}: {} (the pipeline will try to delete the file again on a restart)", path.display(), e); + log::warn!("Unable to remove old-checkpoint file {}: {} (the pipeline will try to delete the file again on a restart)", path.display(), e); } } } @@ -250,10 +250,10 @@ impl Checkpointer { ); match fs::remove_file(file) { Ok(_) => { - tracing::debug!("Removed file {}", file.as_ref().display()); + log::debug!("Removed file {}", file.as_ref().display()); } Err(e) => { - tracing::warn!("Unable to remove old-checkpoint file {}: {} (the pipeline will try to delete the file again on a restart)", file.as_ref().display(), e); + log::warn!("Unable to remove old-checkpoint file {}: {} (the pipeline will try to delete the file again on a restart)", file.as_ref().display(), e); } } } @@ -266,7 +266,7 @@ impl Checkpointer { assert_ne!(cpm.uuid, Uuid::nil()); let checkpoint_dir = self.storage_path.join(cpm.uuid.to_string()); if !checkpoint_dir.exists() { - tracing::warn!( + log::warn!( "Tried to remove a checkpoint directory '{}' that doesn't exist.", checkpoint_dir.display() ); diff --git a/crates/dbsp/src/circuit/runtime.rs b/crates/dbsp/src/circuit/runtime.rs index 4b2d66da11..96160ca036 100644 --- a/crates/dbsp/src/circuit/runtime.rs +++ b/crates/dbsp/src/circuit/runtime.rs @@ -848,7 +848,7 @@ impl RuntimeHandle { match st { StorageLocation::Temporary(path) => { if std::thread::panicking() || did_runtime_panic { - tracing::info!("Preserved runtime storage at: {:?} due to panic", path); + log::info!("Preserved runtime storage at: {:?} due to panic", path); } else { let _ = std::fs::remove_dir_all(path); } diff --git a/crates/dbsp/src/error.rs b/crates/dbsp/src/error.rs index 9e563cf447..55abfa1b26 100644 --- a/crates/dbsp/src/error.rs +++ b/crates/dbsp/src/error.rs @@ -1,5 +1,6 @@ use crate::{storage::backend::StorageError, RuntimeError, SchedulerError}; use anyhow::Error as AnyError; +use log::Level; use serde::{ser::SerializeStruct, Serialize, Serializer}; use std::{ borrow::Cow, @@ -7,12 +8,11 @@ use std::{ fmt::{Display, Error as FmtError, Formatter}, io::Error as IOError, }; -use tracing::Level; pub trait DetailedError: StdError + Serialize { fn error_code(&self) -> Cow<'static, str>; fn log_level(&self) -> Level { - Level::ERROR + Level::Error } } diff --git a/crates/dbsp/src/storage/backend/mod.rs b/crates/dbsp/src/storage/backend/mod.rs index 8af2ecdf1b..5a88e6d9a4 100644 --- a/crates/dbsp/src/storage/backend/mod.rs +++ b/crates/dbsp/src/storage/backend/mod.rs @@ -20,11 +20,11 @@ use std::{ }, }; +use log::{trace, warn}; use pipeline_types::config::StorageCacheConfig; use serde::{ser::SerializeStruct, Serialize, Serializer}; use tempfile::TempDir; use thiserror::Error; -use tracing::{trace, warn}; use uuid::Uuid; use crate::storage::buffer_cache::FBuf; diff --git a/crates/dbsp/src/storage/dirlock/mod.rs b/crates/dbsp/src/storage/dirlock/mod.rs index 4db7126d91..ca69bcea60 100644 --- a/crates/dbsp/src/storage/dirlock/mod.rs +++ b/crates/dbsp/src/storage/dirlock/mod.rs @@ -3,12 +3,12 @@ //! Makes sure we don't accidentally run multiple instances of the program //! using the same data directory. +use log::{debug, warn}; use std::fs::{File, OpenOptions}; use std::io::{self, Error as IoError, ErrorKind, Read, Write}; use std::os::fd::AsRawFd; use std::path::{Path, PathBuf}; use sysinfo::{Pid, System}; -use tracing::{debug, warn}; use crate::storage::backend::StorageError; @@ -110,12 +110,12 @@ impl LockedDirectory { // The pidfile is ours, just leave it as is. } else { // The process doesn't exist, so we can safely overwrite the pidfile. - tracing::debug!("Found stale pidfile: {}", pid_file.display()); + log::debug!("Found stale pidfile: {}", pid_file.display()); } } else { // If the pidfile is corrupt, we won't take ownership of the storage dir until // the user fixes it. - tracing::error!( + log::error!( "Invalid pidfile contents: '{}' in {}, pipeline refused to take ownership of storage dir.", contents, pid_file.display(), diff --git a/crates/dbsp/src/storage/mod.rs b/crates/dbsp/src/storage/mod.rs index d44bffa339..532eba0d03 100644 --- a/crates/dbsp/src/storage/mod.rs +++ b/crates/dbsp/src/storage/mod.rs @@ -11,10 +11,10 @@ pub mod file; mod test; use fdlimit::{raise_fd_limit, Outcome::LimitRaised}; +use log::{info, warn}; use std::fs::OpenOptions; use std::io::Write; use std::path::{Path, PathBuf}; -use tracing::{info, warn}; use crate::{Error, Runtime}; use std::sync::Once; diff --git a/crates/dbsp/src/trace/spine_async/merger.rs b/crates/dbsp/src/trace/spine_async/merger.rs index aab0927251..3c64b32a1e 100644 --- a/crates/dbsp/src/trace/spine_async/merger.rs +++ b/crates/dbsp/src/trace/spine_async/merger.rs @@ -55,7 +55,7 @@ impl BatchMerger { Err(e) => { // We dropped all references to the recv channel, this means // the circuit was destroyed, we can exit the compactor thread. - tracing::trace!( + log::trace!( "exiting compactor thread due to rx error on channel: {:?}", e ); From f6bbf78a832f12049221cdc8c1e4b6791617431a Mon Sep 17 00:00:00 2001 From: Gerd Zellweger Date: Sun, 7 Jul 2024 13:47:53 +0200 Subject: [PATCH 11/18] Add minitrace to dbsp and adapters. Signed-off-by: Gerd Zellweger --- Cargo.lock | 115 ++++++++++++++++++ crates/adapters/Cargo.toml | 2 + crates/adapters/src/server/mod.rs | 13 ++ crates/dbsp/Cargo.toml | 1 + crates/dbsp/src/circuit/circuit_builder.rs | 1 - crates/dbsp/src/circuit/dbsp_handle.rs | 28 +++-- .../src/operator/dynamic/aggregate/mod.rs | 4 + .../operator/dynamic/communication/gather.rs | 5 + .../dbsp/src/operator/dynamic/consolidate.rs | 3 + crates/dbsp/src/operator/dynamic/distinct.rs | 5 + .../dbsp/src/operator/dynamic/filter_map.rs | 11 +- crates/dbsp/src/operator/dynamic/group/mod.rs | 2 + crates/dbsp/src/operator/dynamic/index.rs | 5 +- .../dbsp/src/operator/dynamic/input_upsert.rs | 2 + crates/dbsp/src/operator/dynamic/join.rs | 4 + .../dbsp/src/operator/dynamic/join_range.rs | 2 + .../dbsp/src/operator/dynamic/neighborhood.rs | 3 + crates/dbsp/src/operator/dynamic/sample.rs | 3 + crates/dbsp/src/operator/dynamic/semijoin.rs | 6 +- .../radix_tree/partitioned_tree_aggregate.rs | 2 + .../time_series/radix_tree/tree_aggregate.rs | 2 + .../dynamic/time_series/rolling_aggregate.rs | 2 + .../operator/dynamic/time_series/window.rs | 2 + crates/dbsp/src/operator/dynamic/trace.rs | 7 ++ crates/dbsp/src/operator/dynamic/upsert.rs | 2 + crates/pipeline-types/src/config.rs | 18 +++ crates/pipeline_manager/src/db/test.rs | 6 + openapi.json | 18 +++ 28 files changed, 256 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 59d6971bed..5bd5644617 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2532,6 +2532,12 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "cache-padded" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "981520c98f422fcc584dc1a95c334e6953900b9106bc47a9839b81790009eb21" + [[package]] name = "cached" version = "0.38.0" @@ -3207,6 +3213,16 @@ dependencies = [ "memchr", ] +[[package]] +name = "ctor" +version = "0.1.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d2301688392eb071b0bf1a37be05c469d3cc4dbbd95df672fe28ab021e6a096" +dependencies = [ + "quote 1.0.36", + "syn 1.0.109", +] + [[package]] name = "cty" version = "0.2.2" @@ -3720,6 +3736,7 @@ dependencies = [ "log", "metrics 0.22.3", "mimalloc-rust-sys", + "minitrace", "nix 0.27.1", "num", "num-derive 0.4.2", @@ -3803,6 +3820,8 @@ dependencies = [ "metrics-exporter-prometheus", "metrics-util 0.17.0", "mime", + "minitrace", + "minitrace-jaeger", "mockall", "num-bigint", "num-derive 0.3.3", @@ -6067,6 +6086,44 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" +[[package]] +name = "minitrace" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "197d538cd69839d49a593c8c72df44291b0ea3296ecc0c85529002c53c8fbc6f" +dependencies = [ + "minitrace-macro", + "minstant", + "once_cell", + "parking_lot 0.12.3", + "pin-project", + "rand 0.8.5", + "rtrb", +] + +[[package]] +name = "minitrace-jaeger" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e63612364589c56423b9d3d08083adfadc44570158b49bc76a10562214785d2" +dependencies = [ + "log", + "minitrace", + "thrift_codec", +] + +[[package]] +name = "minitrace-macro" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "14efd4b574325fcb981bce1ac700b9ccf071ec2eb94f7a6a6b583a84f228ba47" +dependencies = [ + "proc-macro-error", + "proc-macro2 1.0.86", + "quote 1.0.36", + "syn 1.0.109", +] + [[package]] name = "miniz_oxide" version = "0.7.4" @@ -6076,6 +6133,16 @@ dependencies = [ "adler", ] +[[package]] +name = "minstant" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fb9b5c752f145ac5046bccc3c4f62892e3c950c1d1eab80c5949cd68a2078db" +dependencies = [ + "ctor", + "web-time", +] + [[package]] name = "mio" version = "0.8.11" @@ -8125,6 +8192,15 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "rtrb" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "99e704dd104faf2326a320140f70f0b736d607c1caa1b1748a6c568a79819109" +dependencies = [ + "cache-padded", +] + [[package]] name = "rust-embed" version = "8.4.0" @@ -9398,6 +9474,16 @@ dependencies = [ "ordered-float 2.10.1", ] +[[package]] +name = "thrift_codec" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83d957f535b242b91aa9f47bde08080f9a6fef276477e55b0079979d002759d5" +dependencies = [ + "byteorder", + "trackable", +] + [[package]] name = "tikv-jemalloc-ctl" version = "0.5.4" @@ -9776,6 +9862,25 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "trackable" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b15bd114abb99ef8cee977e517c8f37aee63f184f2d08e3e6ceca092373369ae" +dependencies = [ + "trackable_derive", +] + +[[package]] +name = "trackable_derive" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ebeb235c5847e2f82cfe0f07eb971d1e5f6804b18dac2ae16349cc604380f82f" +dependencies = [ + "quote 1.0.36", + "syn 1.0.109", +] + [[package]] name = "try-lock" version = "0.2.5" @@ -10188,6 +10293,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "web-time" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + [[package]] name = "webpki" version = "0.22.4" diff --git a/crates/adapters/Cargo.toml b/crates/adapters/Cargo.toml index cb7a077d73..5378a2a29a 100644 --- a/crates/adapters/Cargo.toml +++ b/crates/adapters/Cargo.toml @@ -83,6 +83,8 @@ metrics = "0.23" metrics-util = "0.17" metrics-exporter-prometheus = "0.15.1" ordered-float = { version = "4.2.0", features = ["serde"] } +minitrace = { version = "0.6", features = ["enable"] } +minitrace-jaeger = { version = "0.6" } [target.'cfg(target_os = "linux")'.dependencies] jemalloc_pprof = "0.1.0" diff --git a/crates/adapters/src/server/mod.rs b/crates/adapters/src/server/mod.rs index 1e536acba1..d6d7709455 100644 --- a/crates/adapters/src/server/mod.rs +++ b/crates/adapters/src/server/mod.rs @@ -23,6 +23,7 @@ use dbsp::circuit::CircuitConfig; use dbsp::operator::sample::MAX_QUANTILES; use env_logger::Env; use log::{debug, error, info, warn}; +use minitrace::collector::Config; use pipeline_types::{format::json::JsonFlavor, transport::http::EgressMode}; use pipeline_types::{query::OutputQuery, transport::http::SERVER_PORT_FILE}; use serde::Deserialize; @@ -382,6 +383,18 @@ where }); let _ = loginit_sender.send(()); + if config.global.tracing { + let reporter = minitrace_jaeger::JaegerReporter::new( + config.global.tracing_endpoint_jaeger.parse().unwrap(), + config + .name + .clone() + .unwrap_or("unknown pipeline".to_string()), + ) + .unwrap(); + minitrace::set_reporter(reporter, Config::default()); + } + *state.metadata.write().unwrap() = match &args.metadata_file { None => String::new(), Some(metadata_file) => { diff --git a/crates/dbsp/Cargo.toml b/crates/dbsp/Cargo.toml index d2903102a9..3a1fa5f9d4 100644 --- a/crates/dbsp/Cargo.toml +++ b/crates/dbsp/Cargo.toml @@ -75,6 +75,7 @@ static_assertions = "1.1.0" lazy_static = "1.4.0" dashmap = "5" zip = "0.6.2" +minitrace = "0.6" [dependencies.time] version = "0.3.20" diff --git a/crates/dbsp/src/circuit/circuit_builder.rs b/crates/dbsp/src/circuit/circuit_builder.rs index ea8f907474..5d94e9ee1e 100644 --- a/crates/dbsp/src/circuit/circuit_builder.rs +++ b/crates/dbsp/src/circuit/circuit_builder.rs @@ -4517,7 +4517,6 @@ impl CircuitHandle { pub fn step(&self) -> Result<(), SchedulerError> { // TODO: Add a runtime check to prevent re-entering this method from an // operator. - self.executor.run(&self.circuit) } diff --git a/crates/dbsp/src/circuit/dbsp_handle.rs b/crates/dbsp/src/circuit/dbsp_handle.rs index 42b7ea30b7..64cf49a6e8 100644 --- a/crates/dbsp/src/circuit/dbsp_handle.rs +++ b/crates/dbsp/src/circuit/dbsp_handle.rs @@ -9,7 +9,11 @@ use crossbeam::channel::{bounded, Receiver, Select, Sender, TryRecvError}; use hashbrown::HashMap; use itertools::Either; use metrics::counter; +use minitrace::collector::SpanContext; +use minitrace::local::LocalSpan; +use minitrace::Span; pub use pipeline_types::config::{StorageCacheConfig, StorageConfig}; +use std::sync::Arc; use std::{ collections::HashSet, error::Error as StdError, @@ -328,6 +332,7 @@ impl Runtime { let runtime = Self::run(&config, move || { let worker_index = Runtime::worker_index() - worker_ofs; + let worker_index_str: &'static str = worker_index.to_string().leak(); // Drop all but one channels. This makes sure that if one of the worker panics // or exits, its channel will become disconnected. @@ -352,14 +357,13 @@ impl Runtime { } }; - // TODO: uncomment this when we have support for background compaction. - // let mut moregc = true; - while !Runtime::kill_in_progress() { // Wait for command. match command_receiver.try_recv() { - Ok(Command::Step) => { - //moregc = true; + Ok(Command::Step(span)) => { + let _guard = span.set_local_parent(); + let _worker_span = LocalSpan::enter_with_local_parent("worker-step") + .with_property(|| ("worker", worker_index_str)); let status = circuit.step().map(|_| Response::Unit); // Send response. if status_sender.send(status).is_err() { @@ -413,12 +417,7 @@ impl Runtime { // Nothing to do: do some housekeeping and relinquish the CPU if there's none // left. Err(TryRecvError::Empty) => { - // GC - /*if moregc { - moregc = circuit.gc(); - } else {*/ Runtime::parker().with(|parker| parker.park()); - //} } Err(_) => { break; @@ -471,7 +470,7 @@ impl Runtime { #[derive(Clone)] enum Command { - Step, + Step(Arc), EnableProfiler, DumpProfile, RetrieveProfile, @@ -605,7 +604,12 @@ impl DBSPHandle { pub fn step(&mut self) -> Result<(), DBSPError> { counter!("feldera.dbsp.step").increment(1); self.step_id += 1; - self.broadcast_command(Command::Step, |_, _| {}) + let span = Arc::new( + Span::root("step", SpanContext::random()) + .with_properties(|| [("step", format!("{}", self.step_id))]), + ); + let _guard = span.set_local_parent(); + self.broadcast_command(Command::Step(span), |_, _| {}) } /// Used by the checkpointer to initiate a commit on the circuit. diff --git a/crates/dbsp/src/operator/dynamic/aggregate/mod.rs b/crates/dbsp/src/operator/dynamic/aggregate/mod.rs index 6603f8246f..d19afe1825 100644 --- a/crates/dbsp/src/operator/dynamic/aggregate/mod.rs +++ b/crates/dbsp/src/operator/dynamic/aggregate/mod.rs @@ -1,6 +1,7 @@ //! Aggregation operators. use dyn_clone::{clone_box, DynClone}; +use minitrace::trace; use std::{ any::TypeId, borrow::Cow, @@ -577,6 +578,7 @@ where O: IndexedZSet, Acc: DataTrait + ?Sized, { + #[trace] fn eval(&mut self, i: &Z) -> O { let mut builder = O::Builder::with_capacity(&self.factories, (), i.len()); let mut agg = self.option_output_factory.default_box(); @@ -714,6 +716,7 @@ where /// aggregation logic in the `Aggregator` trait. The second iteration /// is only needed inside nested scopes and can in the future be /// optimized to terminate early. + #[trace] fn eval_key( &mut self, key: &Z::Key, @@ -841,6 +844,7 @@ where Acc: DataTrait + ?Sized, Out: DataTrait + ?Sized, { + #[trace] fn eval(&mut self, delta: &Z, input_trace: &IT) -> Box>> { // println!( // "{}: AggregateIncremental::eval @{:?}\ndelta:{delta}", diff --git a/crates/dbsp/src/operator/dynamic/communication/gather.rs b/crates/dbsp/src/operator/dynamic/communication/gather.rs index 9022bf6916..f73bf4daac 100644 --- a/crates/dbsp/src/operator/dynamic/communication/gather.rs +++ b/crates/dbsp/src/operator/dynamic/communication/gather.rs @@ -11,6 +11,7 @@ use crate::{ use arc_swap::ArcSwap; use crossbeam::atomic::AtomicConsume; use crossbeam_utils::CachePadded; +use minitrace::trace; use std::{ borrow::Cow, marker::PhantomData, @@ -259,11 +260,13 @@ impl SinkOperator for GatherProducer where T: Clone + Send + 'static, { + #[trace] fn eval(&mut self, input: &T) { // Safety: `worker` is guaranteed to be a valid & unique worker index unsafe { self.gather.push(self.worker, input.clone()) } } + #[trace] fn eval_owned(&mut self, input: T) { // Safety: `worker` is guaranteed to be a valid & unique worker index unsafe { self.gather.push(self.worker, input) } @@ -322,6 +325,7 @@ impl SourceOperator for GatherConsumer where T: Batch