Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add CLICKHOUSE_MAX_EXECUTION_TIME, update fetchData #264

Merged
merged 1 commit into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ CLICKHOUSE_HOST=http://localhost:8123
CLICKHOUSE_USER=default
CLICKHOUSE_PASSWORD=
CLICKHOUSE_TZ=
CLICKHOUSE_MAX_EXECUTION_TIME=60
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ To get the project up and running on your local machine, follow these steps:
- `CLICKHOUSE_HOST`: ClickHouse host, for example `http://localhost:8123`
- `CLICKHOUSE_USER`: ClickHouse user with permission to query the `system` database.
- `CLICKHOUSE_PASSWORD`: ClickHouse password for the specified user.
- `CLICKHOUSE_TIMEOUT`: Timeout for ClickHouse queries in milliseconds. Default is `100000`.
- `CLICKHOUSE_MAX_EXECUTION_TIME`: [`max_execution_time`](https://clickhouse.com/docs/en/operations/settings/query-complexity#max-execution-time) timeout in seconds. Default is `10`.
- `CLICKHOUSE_TZ`: ClickHouse server timezone. Default: `''`.
4. Run the development server with `npm run dev` or `yarn dev`
5. Open [http://localhost:3000](http://localhost:3000) in your browser to see the dashboard.
Expand All @@ -50,6 +50,7 @@ docker run -it \
-e CLICKHOUSE_USER='default' \
-e CLICKHOUSE_PASSWORD='' \
-e CLICKHOUSE_TZ='Asia/Ho_Chi_Minh' \
-e CLICKHOUSE_MAX_EXECUTION_TIME='15' \
--name clickhouse-monitoring \
ghcr.io/duyet/clickhouse-monitoring:main
```
Expand All @@ -71,6 +72,8 @@ env:
value: ''
- name: CLICKHOUSE_TZ
value: 'Asia/Ho_Chi_Minh'
- name: CLICKHOUSE_MAX_EXECUTION_TIME
value: '15'
EOF

helm install -f values.yaml clickhouse-monitoring-release duyet/clickhouse-monitoring
Expand Down
7 changes: 6 additions & 1 deletion app/[query]/table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,15 @@ export default async function Table({
...config.defaultParams,
...validQueryParamsObj,
}
const data = await fetchData<RowData[]>({
const { data } = await fetchData<RowData[]>({
query: sql,
format: 'JSONEachRow',
query_params: queryParams,
clickhouse_settings: {
// The main data table takes longer to load.
max_execution_time: 300,
...config.clickhouseSettings,
},
})

return <DataTable title={title} config={config} data={data} />
Expand Down
5 changes: 3 additions & 2 deletions app/api/timezone/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ export const revalidate = false

export async function GET() {
try {
const resp = await fetchData<{ tz: string }[]>({
const { data } = await fetchData<{ tz: string }[]>({
query: 'SELECT timezone() as tz',
})

return NextResponse.json({
tz: resp[0].tz,
tz: data[0].tz,
})
} catch {
return NextResponse.json({}, { status: 500 })
Expand Down
5 changes: 4 additions & 1 deletion app/api/version/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import { fetchData } from '@/lib/clickhouse'

export async function GET() {
try {
const clickhouse = await fetchData({ query: 'SELECT version()' })
const { data: clickhouse } = await fetchData({
query: 'SELECT version() as version',
})

return NextResponse.json({
ui: packageInfo.version,
clickhouse,
Expand Down
10 changes: 4 additions & 6 deletions app/clusters/[cluster]/breadcrumb.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@ interface Props {
}

export async function ClusterListBreadcrumb({ cluster }: Props) {
let clusters: Row[] = []

try {
// Lists cluster names.
clusters = await fetchDataWithCache()<Row[]>({ query: config.sql })
const { data } = await fetchDataWithCache()<Row[]>({ query: config.sql })

if (!clusters.length) {
if (!data.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Potential null check issue

Consider adding a null check for data before accessing its length property to avoid potential runtime errors.

return (
<ErrorAlert
title="Message"
message="No cluster found on system.clusters"
/>
)
}

return <Internal cluster={cluster} clusters={data} />
} catch (e: any) {
return (
<ErrorAlert
Expand All @@ -46,8 +46,6 @@ export async function ClusterListBreadcrumb({ cluster }: Props) {
/>
)
}

return <Internal cluster={cluster} clusters={clusters} />
}

export function ClusterListBreadcrumbSkeleton({ cluster }: Props) {
Expand Down
6 changes: 3 additions & 3 deletions app/clusters/[cluster]/count-across-replicas/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface PageProps {
}

export default async function Page({ params: { cluster } }: PageProps) {
const replicas = await fetchData<{ replica: string }[]>({
const { data: replicas } = await fetchData<{ replica: string }[]>({
query: `SELECT hostName() as replica FROM clusterAllReplicas({cluster: String}) ORDER BY 1`,
query_params: { cluster },
})
Expand Down Expand Up @@ -47,7 +47,7 @@ export default async function Page({ params: { cluster } }: PageProps) {
},
}

const rows = await fetchData<
const { data } = await fetchData<
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consistent destructuring

Ensure consistent destructuring of the response object across all fetchData calls for better readability and maintainability.

Suggested change
const { data } = await fetchData<
const { data: rows } = await fetchData<

{
table: string
[replica: string]: string | number
Expand All @@ -61,7 +61,7 @@ export default async function Page({ params: { cluster } }: PageProps) {
<DataTable
title={`Total rows of active parts across replicas in the '${cluster}' cluster`}
config={config}
data={rows}
data={data}
/>
)
}
6 changes: 3 additions & 3 deletions app/clusters/[cluster]/parts-across-replicas/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface PageProps {
}

export default async function Page({ params: { cluster } }: PageProps) {
const replicas = await fetchData<{ replica: string }[]>({
const { data: replicas } = await fetchData<{ replica: string }[]>({
query: `SELECT hostName() as replica FROM clusterAllReplicas({cluster: String}) ORDER BY 1`,
query_params: { cluster },
})
Expand Down Expand Up @@ -47,7 +47,7 @@ export default async function Page({ params: { cluster } }: PageProps) {
},
}

const rows = await fetchData<
const { data } = await fetchData<
{
table: string
[replica: string]: string | number
Expand All @@ -61,7 +61,7 @@ export default async function Page({ params: { cluster } }: PageProps) {
<DataTable
title={`Count of active parts across replicas in the '${cluster}' cluster`}
config={config}
data={rows}
data={data}
/>
)
}
4 changes: 2 additions & 2 deletions app/clusters/[cluster]/replicas-status/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ interface PageProps {
}

export default async function Page({ params: { cluster } }: PageProps) {
const tables = await fetchData<Row[]>({
const { data } = await fetchData<Row[]>({
query: config.sql,
query_params: { cluster },
})
Expand All @@ -21,7 +21,7 @@ export default async function Page({ params: { cluster } }: PageProps) {
<DataTable
title={`Row counts across '${cluster}' cluster`}
config={config}
data={tables}
data={data}
topRightToolbarExtras={<TopRightToolbarExtras cluster={cluster} />}
/>
)
Expand Down
4 changes: 2 additions & 2 deletions app/clusters/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { config, type Row } from './config'

// Redirects to the first database.
export default async function ClustersPage() {
const tables = await fetchData<Row[]>({ query: config.sql })
const { data } = await fetchData<Row[]>({ query: config.sql })

return <DataTable config={config} data={tables} />
return <DataTable config={config} data={data} />
}
2 changes: 1 addition & 1 deletion app/dashboard/render-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const RenderChart = async ({
className,
chartClassName,
}: RenderChartProps) => {
const data = await fetchData<{ [key: string]: string | number }[]>({
const { data } = await fetchData<{ [key: string]: string | number }[]>({
query,
query_params: params,
})
Expand Down
4 changes: 2 additions & 2 deletions app/dashboard/seeding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const migrateSettings = async () => {
]

for (const seed of seeds) {
const exists = await fetchData<TableSettingsRow[]>({
const { data: exists } = await fetchData<TableSettingsRow[]>({
query: `
SELECT * FROM ${TABLE_SETTINGS}
FINAL
Expand Down Expand Up @@ -99,7 +99,7 @@ const migrateDashboard = async () => {
]

for (const seed of seeds) {
const exists = await fetchData<TableChartsRow[]>({
const { data: exists } = await fetchData<TableChartsRow[]>({
query: `SELECT * FROM ${TABLE_CHARTS} FINAL WHERE title = '${seed.title}'`,
})

Expand Down
12 changes: 7 additions & 5 deletions app/dashboard/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ import { seeding } from './seeding'
export const getCustomDashboards = async () => {
await seeding()

const dashboards = await fetchData<TableChartsRow[]>({
const q1 = fetchData<TableChartsRow[]>({
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (performance): Potential performance improvement

Consider using Promise.all directly without intermediate variables q1 and q2 to potentially improve readability and performance.

Suggested change
const q1 = fetchData<TableChartsRow[]>({
await Promise.all([
fetchData<TableChartsRow[]>({
query: `SELECT * FROM ${TABLE_CHARTS} FINAL ORDER BY ordering ASC`,
})
]);

query: `SELECT * FROM ${TABLE_CHARTS} FINAL ORDER BY ordering ASC`,
})
const settings = await fetchData<TableSettingsRow[]>({
}).then((res) => res.data)
const q2 = fetchData<TableSettingsRow[]>({
query: `SELECT * FROM ${TABLE_SETTINGS} FINAL`,
})
}).then((res) => res.data)

const [dashboards, settings] = await Promise.all([q1, q2])

return { settings, dashboards }
return { dashboards, settings }
}

export async function updateSettingParams(
Expand Down
42 changes: 25 additions & 17 deletions app/database/[database]/[table]/@mergetree/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,37 @@ export default async function MergeTree({
const engine = await engineType(database, table)
if (engine.includes('MergeTree') === false) return <></>

const columns = await fetchDataWithCache()<Row[]>({
const { data: columns } = await fetchDataWithCache()<Row[]>({
query: config.sql,
query_params: {
database,
table,
},
})

let description = ''
return (
<DataTable
title={`Table: ${database}.${table}`}
description={<Description database={database} table={table} />}
toolbarExtras={<Extras database={database} table={table} />}
topRightToolbarExtras={
<TopRightToolbarExtras database={database} table={table} />
}
config={config}
data={columns}
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Potential issue with data structure

Ensure that columns is structured correctly for the DataTable component. The change from fetchDataWithCache() to destructuring might have altered the expected data format.

/>
)
}

async function Description({
database,
table,
}: {
database: string
table: string
}) {
try {
const raw = await fetchDataWithCache()<{ comment: string }[]>({
const { data } = await fetchDataWithCache()<{ comment: string }[]>({
query: `
SELECT comment
FROM system.tables
Expand All @@ -42,23 +62,11 @@ export default async function MergeTree({
query_params: { database, table },
})

description = raw?.[0]?.comment || ''
return data?.[0]?.comment || ''
} catch (e) {
console.error('Error fetching table description', e)
return ''
}

return (
<DataTable
title={`Table: ${database}.${table}`}
description={description}
toolbarExtras={<Extras database={database} table={table} />}
topRightToolbarExtras={
<TopRightToolbarExtras database={database} table={table} />
}
config={config}
data={columns}
/>
)
}

const TopRightToolbarExtras = ({
Expand Down
6 changes: 3 additions & 3 deletions app/database/[database]/[table]/engine-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { fetchDataWithCache } from '@/lib/clickhouse'

export const engineType = async (database: string, table: string) => {
try {
const resp = await fetchDataWithCache()<{ engine: string }[]>({
const { data } = await fetchDataWithCache()<{ engine: string }[]>({
query: `
SELECT engine
FROM system.tables
Expand All @@ -12,9 +12,9 @@ export const engineType = async (database: string, table: string) => {
query_params: { database, table },
})

return resp?.[0]?.engine || ''
return data?.[0]?.engine || ''
} catch (error) {
console.error(error)
console.error(`Fetch engine type for ${database}.${table} error:`, error)
return ''
}
}
10 changes: 8 additions & 2 deletions app/database/[database]/[table]/extras/alternative-tables.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,16 @@ export async function AlternativeTables({
}: AlternativeTablesProps) {
let anotherTables: { name: string }[] = []
try {
anotherTables = await fetchData<{ name: string }[]>({
query: `SELECT name FROM system.tables WHERE database = {database: String}`,
const res = await fetchData<{ name: string }[]>({
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Variable naming consistency

Consider renaming res to response for better readability and consistency with other parts of the codebase.

Suggested change
const res = await fetchData<{ name: string }[]>({
const response = await fetchData<{ name: string }[]>({

query: `
SELECT name
FROM system.tables
WHERE database = {database: String}
`,
query_params: { database },
})

anotherTables = res.data
} catch (error) {
console.log(error)

Expand Down
15 changes: 7 additions & 8 deletions app/database/[database]/[table]/extras/running-queries-count.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ export async function RunningQueriesCount({
database,
table,
}: RunningQueriesProps) {
let data: { count: number }[] = []
try {
data = await fetchData<{ count: number }[]>({
const { data } = await fetchData<{ count: number }[]>({
query: `
SELECT COUNT() as count
FROM system.processes
Expand All @@ -24,14 +23,14 @@ export async function RunningQueriesCount({
table,
},
})

if (!data?.length) {
return null
}

return <Badge variant="outline">{data[0].count || 0}</Badge>
} catch (error) {
console.error(error)
return null
}

if (!data?.length) {
return null
}

return <Badge variant="outline">{data[0].count || 0}</Badge>
}
Loading