You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/09/26 00:53:44 UTC
[GitHub] [druid] vogievetsky opened a new pull request #10438: Web console: Display compaction status
vogievetsky opened a new pull request #10438:
URL: https://github.com/apache/druid/pull/10438
This PR adds a column to display the (new) compaction status API
This PR also:
- Adds suggestion options for the `skipOffsetFromLatest` parameter
- Improves error cursor placement in the JsonInput
- adds an alt + click option to the `...` button allowing for a secret undocumented API option to run compaction immediately. This is very useful for debugging and the copy is made suitably scary to ensure users are dissuade from using this option.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #10438:
URL: https://github.com/apache/druid/pull/10438#issuecomment-700347608
I think this is good to go, and some of the tests that were addressed in review should be added in a subsequent PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #10438:
URL: https://github.com/apache/druid/pull/10438#discussion_r496283127
##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -131,9 +132,70 @@ function twoLines(line1: string, line2: string) {
);
}
+function progress(done: number, awaiting: number): number {
+ const d = done + awaiting;
+ if (!d) return 0;
+ return done / d;
+}
+
+function capitalizeFirst(str: string): string {
+ return str.slice(0, 1).toUpperCase() + str.slice(1).toLowerCase();
+}
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #10438:
URL: https://github.com/apache/druid/pull/10438#discussion_r496187642
##########
File path: web-console/src/components/more-button/more-button.tsx
##########
@@ -18,14 +18,19 @@
import { Button, Menu, Popover, Position } from '@blueprintjs/core';
import { IconNames } from '@blueprintjs/icons';
-import React from 'react';
+import React, { useState } from 'react';
+
+type OpenState = 'open' | 'alt-open';
export interface MoreButtonProps {
- children: React.ReactNode;
+ children: React.ReactNode | React.ReactNode[];
+ altExtra?: React.ReactNode;
Review comment:
What do you think about adding another snapshot test that sets the new `altExtra` prop?
##########
File path: web-console/src/utils/general.tsx
##########
@@ -231,6 +231,10 @@ export function formatMegabytes(n: number): string {
return numeral(n / 1048576).format('0,0.0');
}
+export function formatPercent(n: number): string {
+ return (n * 100).toFixed(2) + '%';
+}
Review comment:
Missing unit test in `general.spec.ts`
##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -329,18 +393,25 @@ GROUP BY 1`;
const rulesResp = await axios.get('/druid/coordinator/v1/rules');
const rules = rulesResp.data;
- const compactionResp = await axios.get('/druid/coordinator/v1/config/compaction');
- const compaction = lookupBy(
- compactionResp.data.compactionConfigs,
- (c: any) => c.dataSource,
+ const compactionConfigsResp = await axios.get('/druid/coordinator/v1/config/compaction');
+ const compactionConfigs = lookupBy(
+ compactionConfigsResp.data.compactionConfigs || [],
+ (c: CompactionConfig) => c.dataSource,
+ );
+
+ const compactionStatusesResp = await axios.get('/druid/coordinator/v1/compaction/status');
+ const compactionStatuses = lookupBy(
+ compactionStatusesResp.data.latestStatus || [],
+ (c: CompactionStatus) => c.dataSource,
Review comment:
This will be hard to unit test since it requires either a real or mock druid cluster. One option to make it more testable is to restructure the code to inject something like a compaction manager dependency via the `DatasourcesView` constructor. That way, the unit tests can have a compaction manager mock where the API responses are stubbed. https://github.com/apache/druid/pull/9956 used this approach to test `SegmentTimeline` with a mock for `QueryManager`.
##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -552,7 +634,32 @@ GROUP BY 1`;
);
}
- private saveRules = async (datasource: string, rules: any[], comment: string) => {
+ renderForceCompactAction() {
+ const { showForceCompact } = this.state;
+ if (!showForceCompact) return;
+
+ return (
+ <AsyncActionDialog
+ action={async () => {
+ const resp = await axios.post(`/druid/coordinator/v1/compaction/compact`, {});
+ return resp.data;
+ }}
+ confirmButtonText="Force compaction run"
+ successText="Out of band compaction run has been initiated"
+ failText="Could not force compaction"
+ intent={Intent.DANGER}
+ onClose={() => {
+ this.setState({ showForceCompact: false });
+ }}
+ >
+ <p>Are you sure you want to force a compaction run?</p>
+ <p>This functionality only exists for debugging and testing reasons.</p>
+ <p>If you are running it in production you are doing something wrong.</p>
+ </AsyncActionDialog>
Review comment:
Currently this is hard to test. The autocompaction E2E test could possibly be modified to use this instead of calling the trigger compaction API directly.
##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -131,9 +132,70 @@ function twoLines(line1: string, line2: string) {
);
}
+function progress(done: number, awaiting: number): number {
+ const d = done + awaiting;
+ if (!d) return 0;
+ return done / d;
+}
+
+function capitalizeFirst(str: string): string {
+ return str.slice(0, 1).toUpperCase() + str.slice(1).toLowerCase();
+}
Review comment:
May be worth adding unit tests for all these new utility methods
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #10438:
URL: https://github.com/apache/druid/pull/10438#discussion_r496290706
##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -329,18 +393,25 @@ GROUP BY 1`;
const rulesResp = await axios.get('/druid/coordinator/v1/rules');
const rules = rulesResp.data;
- const compactionResp = await axios.get('/druid/coordinator/v1/config/compaction');
- const compaction = lookupBy(
- compactionResp.data.compactionConfigs,
- (c: any) => c.dataSource,
+ const compactionConfigsResp = await axios.get('/druid/coordinator/v1/config/compaction');
+ const compactionConfigs = lookupBy(
+ compactionConfigsResp.data.compactionConfigs || [],
+ (c: CompactionConfig) => c.dataSource,
+ );
+
+ const compactionStatusesResp = await axios.get('/druid/coordinator/v1/compaction/status');
+ const compactionStatuses = lookupBy(
+ compactionStatusesResp.data.latestStatus || [],
+ (c: CompactionStatus) => c.dataSource,
Review comment:
The testing refactor I suggested, could be fairly involved as it'd be good to add tests for the parts of the datasources view that are not being modified by this PR. Perhaps it'd be best to have a separate PR that has tests for all the parts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #10438:
URL: https://github.com/apache/druid/pull/10438#discussion_r496105362
##########
File path: web-console/src/components/json-input/json-input.tsx
##########
@@ -80,12 +80,11 @@ export const JsonInput = React.memo(function JsonInput(props: JsonInputProps) {
const aceEditor = useRef<Editor | undefined>();
useEffect(() => {
- if (!deepEqual(value, internalValue.value)) {
- setInternalValue({
- value,
- stringified: stringifyJson(value),
- });
- }
+ if (deepEqual(value, internalValue.value)) return;
+ setInternalValue({
+ value,
+ stringified: stringifyJson(value),
+ });
Review comment:
there is no logical change here... just wanted to have less nesting
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #10438:
URL: https://github.com/apache/druid/pull/10438#discussion_r496283372
##########
File path: web-console/src/utils/general.tsx
##########
@@ -231,6 +231,10 @@ export function formatMegabytes(n: number): string {
return numeral(n / 1048576).format('0,0.0');
}
+export function formatPercent(n: number): string {
+ return (n * 100).toFixed(2) + '%';
+}
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #10438:
URL: https://github.com/apache/druid/pull/10438#discussion_r496283305
##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -552,7 +634,32 @@ GROUP BY 1`;
);
}
- private saveRules = async (datasource: string, rules: any[], comment: string) => {
+ renderForceCompactAction() {
+ const { showForceCompact } = this.state;
+ if (!showForceCompact) return;
+
+ return (
+ <AsyncActionDialog
+ action={async () => {
+ const resp = await axios.post(`/druid/coordinator/v1/compaction/compact`, {});
+ return resp.data;
+ }}
+ confirmButtonText="Force compaction run"
+ successText="Out of band compaction run has been initiated"
+ failText="Could not force compaction"
+ intent={Intent.DANGER}
+ onClose={() => {
+ this.setState({ showForceCompact: false });
+ }}
+ >
+ <p>Are you sure you want to force a compaction run?</p>
+ <p>This functionality only exists for debugging and testing reasons.</p>
+ <p>If you are running it in production you are doing something wrong.</p>
+ </AsyncActionDialog>
Review comment:
I am 👍 to modify the e2e test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky merged pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
vogievetsky merged pull request #10438:
URL: https://github.com/apache/druid/pull/10438
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #10438:
URL: https://github.com/apache/druid/pull/10438#discussion_r499094067
##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -552,7 +634,32 @@ GROUP BY 1`;
);
}
- private saveRules = async (datasource: string, rules: any[], comment: string) => {
+ renderForceCompactAction() {
+ const { showForceCompact } = this.state;
+ if (!showForceCompact) return;
+
+ return (
+ <AsyncActionDialog
+ action={async () => {
+ const resp = await axios.post(`/druid/coordinator/v1/compaction/compact`, {});
+ return resp.data;
+ }}
+ confirmButtonText="Force compaction run"
+ successText="Out of band compaction run has been initiated"
+ failText="Could not force compaction"
+ intent={Intent.DANGER}
+ onClose={() => {
+ this.setState({ showForceCompact: false });
+ }}
+ >
+ <p>Are you sure you want to force a compaction run?</p>
+ <p>This functionality only exists for debugging and testing reasons.</p>
+ <p>If you are running it in production you are doing something wrong.</p>
+ </AsyncActionDialog>
Review comment:
I took a stab at modifying the autocompaction E2E test to use the new trigger compaction UI: https://github.com/apache/druid/pull/10469
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on pull request #10438: Web console: Display compaction status
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #10438:
URL: https://github.com/apache/druid/pull/10438#issuecomment-700432544
thank you for the feedback! Will followup a PR with fancier e2e tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org