Skip to content

feat(dashboard): display metadata for index itself besides index table #21680

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

Merged
merged 1 commit into from
May 6, 2025

Conversation

BugenZhao
Copy link
Member

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Previously we only displayed the metadata for the state table of the index, which lacked information about the index itself, espeically the index_items, i.e., expression of each index column.

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • I have checked the Release Timeline and Currently Supported Versions to determine which release branches I need to cherry-pick this PR into.

Documentation

  • My PR needs documentation updates.
Release note

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the dashboard to display metadata for the index itself (including index items) in addition to the index table information. Key changes include:

  • Refactoring the endpoints to separate index table and index item metadata.
  • Adding a new API function to join index table data with the corresponding index items.
  • Updating the UI to display an additional column for index item counts.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/meta/src/dashboard/mod.rs Introduces separate endpoints for index tables and index metadata.
src/meta/src/controller/catalog/list_op.rs Adds a new controller method to list index items from the catalog.
dashboard/pages/indexes.tsx Adds a new column to display index item counts in the dashboard UI.
dashboard/lib/api/streaming.ts Implements the merging logic for index table and index item metadata.
Comments suppressed due to low confidence (2)

src/meta/src/dashboard/mod.rs:153

  • [nitpick] Consider renaming the 'list_indexes' function to 'list_index_items' or similar to clearly differentiate it from the 'list_index_tables' endpoint.
pub async fn list_indexes(Extension(srv): Extension<Service>) -> Result<Json<Vec<Index>>> {

dashboard/pages/indexes.tsx:28

  • [nitpick] The column header 'Index Len' may be ambiguous; consider using a more descriptive label like 'Index Column Count' to better communicate its purpose.
const indexItemsCountColumn: Column<Index> = {

// Join them by id.
return index_tables.flatMap((x) => {
let index = index_items.find((y) => y.id === x.id)
if (index === undefined) {
Copy link
Preview

Copilot AI May 1, 2025

Choose a reason for hiding this comment

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

When merging index table and index item data, a missing matching index item leads to the entry being silently dropped. Consider explicitly handling this case (e.g., logging a warning) to avoid potential data discrepancies.

Suggested change
if (index === undefined) {
if (index === undefined) {
console.warn(`No matching index item found for index table with id: ${x.id}`)

Copilot uses AI. Check for mistakes.

@BugenZhao BugenZhao added this pull request to the merge queue May 6, 2025
Merged via the queue into main with commit d36ffcc May 6, 2025
34 checks passed
@BugenZhao BugenZhao deleted the bz/dashboard-show-index branch May 6, 2025 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants