Skip to content

Conversation

kumarUjjawal
Copy link

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

This PR is related to the issue #6907

What's changed and what's your intention?

As discussed in the issue #6907 the proposal was to Eliminate the export_metrics functionality and related documentation (including the standalone page) and remove the information_schema.runtime_metrics table. I have hunt down all the refrences and removed as per my knowledge.

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@kumarUjjawal kumarUjjawal requested review from MichaelScofield and a team as code owners September 7, 2025 11:24
@github-actions github-actions bot added size/M docs-not-required This change does not impact docs. labels Sep 7, 2025
@WenyXu
Copy link
Member

WenyXu commented Sep 8, 2025

Hi @kumarUjjawal There are some conflicts

@kumarUjjawal
Copy link
Author

Hi @kumarUjjawal There are some conflicts

I will resolve those soon.

@kumarUjjawal
Copy link
Author

@WenyXu can you approve the workflow?

@killme2008 killme2008 changed the title refactor: remove export_metrics feature and related configuration/docs refactor!: remove export_metrics feature and related configuration/docs Sep 9, 2025
@WenyXu
Copy link
Member

WenyXu commented Sep 9, 2025

@WenyXu can you approve the workflow?

Hi @kumarUjjawal, some CI checks have failed. Please follow the CI output for details. For Clippy and rustfmt issues, you can run make clippy and make fmt locally to reproduce them.

@kumarUjjawal
Copy link
Author

@WenyXu can you approve the workflow?

Hi @kumarUjjawal, some CI checks have failed. Please follow the CI output for details. For Clippy and rustfmt issues, you can run make clippy and make fmt locally to reproduce them.

Definitely, I will fix those.

@kumarUjjawal
Copy link
Author

@WenyXu There are mentions of export_metrics in the config also the server crate contains the export_metrics.rs and the self contained tests, should I remove those as well.

@WenyXu
Copy link
Member

WenyXu commented Sep 9, 2025

@WenyXu There are mentions of export_metrics in the config also the server crate contains the export_metrics.rs and the self contained tests, should I remove those as well.

Yes

@github-actions github-actions bot added size/M and removed size/S labels Sep 9, 2025
@evenyag
Copy link
Contributor

evenyag commented Sep 9, 2025

Hi, please fix the CI checks and sign your commits.

@kumarUjjawal
Copy link
Author

Hi, I found three variants of SNAFU error that are defined but never used anywhere, that's why one of the checks was failing, what is your preferences for that, should I remove those?

@kumarUjjawal
Copy link
Author

The thee failing tests doesn't seem to come from my changes, Can you check?
./tests/cases/distributed/common/view/create
./tests/cases/distributed/common/system/information_schema
./tests/cases/distributed/common/show/show_databases_tables

@evenyag
Copy link
Contributor

evenyag commented Sep 10, 2025

Hi, I found three variants of SNAFU error that are defined but never used anywhere, that's why one of the checks was failing, what is your preferences for that, should I remove those?

Sure, it's fine to remove them.

The thee failing tests doesn't seem to come from my changes, Can you check?
./tests/cases/distributed/common/view/create
./tests/cases/distributed/common/system/information_schema
./tests/cases/distributed/common/show/show_databases_tables

They are related to your changes. You should update the sqlness result because you have removed a table from the database.
https://docs.greptime.com/contributor-guide/tests/sqlness-test/

@kumarUjjawal
Copy link
Author

Hi, I found three variants of SNAFU error that are defined but never used anywhere, that's why one of the checks was failing, what is your preferences for that, should I remove those?

Sure, it's fine to remove them.

The thee failing tests doesn't seem to come from my changes, Can you check?
./tests/cases/distributed/common/view/create
./tests/cases/distributed/common/system/information_schema
./tests/cases/distributed/common/show/show_databases_tables

They are related to your changes. You should update the sqlness result because you have removed a table from the database. https://docs.greptime.com/contributor-guide/tests/sqlness-test/

Oh! Thanks for the docs.

@evenyag
Copy link
Contributor

evenyag commented Sep 16, 2025

Hi, the unit test failed.

You also have to update this test:

[export_metrics]
enable = false
write_interval = "30s"

@kumarUjjawal
Copy link
Author

Hi, the unit test failed.

You also have to update this test:

[export_metrics]
enable = false
write_interval = "30s"

Will removing these line fix the issue or do I need to make other changes as well?

@evenyag
Copy link
Contributor

evenyag commented Sep 17, 2025

Hi, the unit test failed.
You also have to update this test:

[export_metrics]
enable = false
write_interval = "30s"

Will removing these line fix the issue or do I need to make other changes as well?

You can run make check to ensure all tests are passed, or check the CI status and fix failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required This change does not impact docs. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants