perf: close issue 4974 by do not delete columns when drop logical region about 100 times faster #5561

yihong0618 · 2025-02-18T08:09:31Z

I hereby agree to the terms of the GreptimeDB CLA.

After some search and learn the region server found maybe we can skip the drop columns in drop logical region
since all data can be drop in phy table I wonder.

because the function get_all or get_all_with_prefix is very expensive, cost almost all the time for the drop database

before this patch

after this patch

the generate data scripts

import psycopg2
import time
from datetime import datetime, timedelta

# --- Configuration ---
DB_HOST = "localhost"
DB_PORT = "4003"
DB_USER = "greptime"
DB_NAME = "public"
SENSORS_PER_LOCATION = 4000
SIMULATION_DURATION_HOURS = 5
INSERT_BATCH_SIZE = 4000




def insert_data_now(simulation_start_time):
    conn = create_connection()
    try:
        # First create physical table
        with conn.cursor() as cur:
            cur.execute("CREATE DATABASE test;")
            cur.execute("use test;")
            physical_table_sql = "CREATE TABLE phy (ts timestamp time index, val double) engine=metric with (\"physical_metric_table\" = \"\");"
            physical2_table_sql = "CREATE TABLE phy2 (ts timestamp time index, val double) engine=metric with (\"physical_metric_table\" = \"\");"
            try:
                cur.execute(physical_table_sql)
                cur.execute(physical2_table_sql)
                conn.commit()
            except Exception as e:
                print(f"Physical table creation error: {e}")

        current_time = simulation_start_time

        with conn.cursor() as cur:
            index = 1
            while current_time <= simulation_start_time + timedelta(seconds=200):
                insert_sql = f"CREATE TABLE t{index} (ts timestamp time index, val double, host string primary key) engine = metric with (\"on_physical_table\" = \"phy\");"
                insert2_sql = f"CREATE TABLE b{index} (ts timestamp time index, val double, host string primary key) engine = metric with (\"on_physical_table\" = \"phy2\");"
                index += 1
                try:
                    cur.execute(insert_sql)
                    cur.execute(insert2_sql)
                    conn.commit()
                except Exception as e:
                    print(f"Table creation error: {e}")
                    continue
                print(f"Created table t{index-1} at {current_time}")
                print(f"Created table b{index-1} at {current_time}")
                current_time += timedelta(seconds=1)
    finally:
        if conn:
            conn.close() # close connect in the thread.

def create_connection():
    """Creates a database connection."""
    conn = psycopg2.connect(
        host=DB_HOST,
        port=DB_PORT,
        user=DB_USER,
        dbname=DB_NAME
    )
    return conn

def main():
    """Main function."""
    simulation_start_time = datetime.utcnow() - timedelta(seconds=200)

    insert_data_now(simulation_start_time=simulation_start_time)

if __name__ == "__main__":
    main()

Refer to a related PR or issue link (optional)

close #4974

What's changed and what's your intention?

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.

coderabbitai · 2025-02-18T08:09:40Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

src/metric-engine/src/metadata_region.rs

yihong0618 · 2025-02-19T13:52:29Z

@fengjiachun @WenyXu done using fast_drop_database_path way
for the ci using my fork repo
and upstream patch link GreptimeTeam/greptime-proto#218

fengjiachun

LGTM, thanks you! @yihong0618

fengjiachun · 2025-02-20T07:07:00Z

@WenyXu PTAL

src/store-api/src/region_request.rs

src/datanode/src/region_server.rs

src/metric-engine/src/engine/drop.rs

src/store-api/src/region_request.rs

src/mito2/src/request.rs

src/metric-engine/src/metadata_region.rs

yihong0618 · 2025-02-21T07:20:13Z

address by use else path way.
and frankly speaking, I prefer the previous implementation in the drop logical function seems no particular issue for that?

WenyXu · 2025-02-21T10:25:31Z

address by use else path way. and frankly speaking, I prefer the previous implementation in the drop logical function seems no particular issue for that?

If all logical tables are force dropped via drop physical region request, we don't need to send drop logical table requests to datanode. By the way, we don't need to handle the (DropTableTarget::Logical, TableRouteValue::Logical(route)) branch anymore; just ignore it.

greptimedb/src/common/meta/src/ddl/drop_database/cursor.rs

Lines 64 to 91 in f6f617d

    
           async fn handle_table( 
        
               &mut self, 
        
               ddl_ctx: &DdlContext, 
        
               ctx: &mut DropDatabaseContext, 
        
               table_name: String, 
        
               table_id: TableId, 
        
               table_route_value: TableRouteValue, 
        
           ) -> Result<(Box<dyn State>, Status)> { 
        
               match (self.target, table_route_value) { 
        
                   (DropTableTarget::Logical, TableRouteValue::Logical(route)) => { 
        
                       let physical_table_id = route.physical_table_id(); 
        
                       let (_, table_route) = ddl_ctx 
        
                           .table_metadata_manager 
        
                           .table_route_manager() 
        
                           .get_physical_table_route(physical_table_id) 
        
                           .await?; 
        
                       Ok(( 
        
                           Box::new(DropDatabaseExecutor::new( 
        
                               table_id, 
        
                               table_id, 
        
                               TableName::new(&ctx.catalog, &ctx.schema, &table_name), 
        
                               table_route.region_routes, 
        
                               self.target, 
        
                           )), 
        
                           Status::executing(true), 
        
                       )) 
        
                   }

yihong0618 · 2025-02-21T11:47:50Z

address by use else path way. and frankly speaking, I prefer the previous implementation in the drop logical function seems no particular issue for that?

If all logical tables are force dropped via drop physical region request, we don't need to send drop logical table requests to datanode. By the way, we don't need to handle the (DropTableTarget::Logical, TableRouteValue::Logical(route)) branch anymore; just ignore it.

greptimedb/src/common/meta/src/ddl/drop_database/cursor.rs

Lines 64 to 91 in f6f617d

async fn handle_table(

&mut self,

ddl_ctx: &DdlContext,

ctx: &mut DropDatabaseContext,

table_name: String,

table_id: TableId,

table_route_value: TableRouteValue,

) -> Result<(Box<dyn State>, Status)> {

match (self.target, table_route_value) {

(DropTableTarget::Logical, TableRouteValue::Logical(route)) => {

let physical_table_id = route.physical_table_id();

let (_, table_route) = ddl_ctx

.table_metadata_manager

.table_route_manager()

.get_physical_table_route(physical_table_id)

.await?;

Ok((

Box::new(DropDatabaseExecutor::new(

table_id,

table_id,

TableName::new(&ctx.catalog, &ctx.schema, &table_name),

table_route.region_routes,

self.target,

)),

Status::executing(true),

))

}

using this way do not check is logical region can be recover? the test_drop_database_recover also failed
@WenyXu

error local

running 1 test
2025-02-21T13:04:34.395303Z  INFO common_telemetry::logging: logs dir = /tmp/__unittest_logs
2025-02-21T13:04:34.395877Z DEBUG common_meta::ddl::table_meta: Allocated region wal options {0: "{\"wal.provider\":\"raft_engine\"}"} for table 1024
2025-02-21T13:04:34.396312Z  WARN common_meta::ddl::create_logical_tables::update_metadata: No physical columns found, leaving the physical table's schema unchanged when creating logical tables
2025-02-21T13:04:34.396379Z  INFO common_meta::ddl::create_logical_tables::update_metadata: Created 1 tables [1025] metadata for physical table 1024
2025-02-21T13:04:34.397300Z DEBUG common_meta::ddl::drop_table::executor: Dropping region 4398046511104(1024, 0) on Datanode Peer { id: 0, addr: "" }
2025-02-21T13:04:34.397335Z DEBUG common_meta::ddl::test_util::datanode_handler: Returning Ok(0) for request: RegionRequest { header: Some(RegionRequestHeader { tracing_context: {}, dbname: "", query_context: None }), body: Some(Drop(DropRequest { region_id: 4398046511104, force_drop_all_logical_tables: true })) }, peer: Peer { id: 0, addr: "" }
2025-02-21T13:04:34.397358Z  INFO common_meta::ddl::drop_database::executor: Table: greptime.public.phy(1024) is dropped
test ddl::tests::drop_database::test_drop_database_recover ... FAILED

failures:

failures:
    ddl::tests::drop_database::test_drop_database_recover

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 248 filtered out; finished in 0.01s

──── TRY 3 STDERR:       common-meta ddl::tests::drop_database::test_drop_database_recover
thread 'ddl::tests::drop_database::test_drop_database_recover' panicked at /Users/hyi/prs/greptimedb/src/common/procedure-test/src/lib.rs:137:5:
procedure 'metasrv-procedure::DropDatabase' did not reach the expected state
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

   RETRY 4/4 [         ] common-meta ddl::tests::drop_database::test_drop_database_recover
        PASS [   0.006s] common-meta key::flow::flow_info::tests::test_key_serialization
        PASS [   0.007s] common-meta key::flow::flow_name::tests::test_key_serialization
        PASS [   0.011s] common-meta key::flow::flow_info::tests::test_key_deserialization
        PASS [   0.008s] common-meta key::flow::flow_name::tests::test_key_start_range
        PASS [   0.007s] common-meta key::flow::flow_route::tests::test_key_deserialization
        PASS [   0.014s] common-meta key::flow::flow_name::tests::test_key_deserialization
        PASS [   0.008s] common-meta key::flow::flow_route::tests::test_key_serialization
        PASS [   0.009s] common-meta key::flow::flow_route::tests::test_key_start_range
        PASS [   0.008s] common-meta key::flow::flownode_flow::tests::test_key_serialization
        PASS [   0.009s] common-meta key::flow::flownode_flow::tests::test_key_deserialization
  TRY 4 FAIL [   0.019s] common-meta ddl::tests::drop_database::test_drop_database_recover
──── TRY 4 STDOUT:       common-meta ddl::tests::drop_database::test_drop_database_recover

running 1 test
2025-02-21T13:04:34.415082Z  INFO common_telemetry::logging: logs dir = /tmp/__unittest_logs
2025-02-21T13:04:34.415863Z DEBUG common_meta::ddl::table_meta: Allocated region wal options {0: "{\"wal.provider\":\"raft_engine\"}"} for table 1024
2025-02-21T13:04:34.416323Z  WARN common_meta::ddl::create_logical_tables::update_metadata: No physical columns found, leaving the physical table's schema unchanged when creating logical tables
2025-02-21T13:04:34.416394Z  INFO common_meta::ddl::create_logical_tables::update_metadata: Created 1 tables [1025] metadata for physical table 1024
2025-02-21T13:04:34.417372Z DEBUG common_meta::ddl::drop_table::executor: Dropping region 4398046511104(1024, 0) on Datanode Peer { id: 0, addr: "" }
test ddl::tests::drop_database::test_drop_database_recover ... FAILED

failures:

failures:
    ddl::tests::drop_database::test_drop_database_recover

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 248 filtered out; finished in 0.01s

──── TRY 4 STDERR:       common-meta ddl::tests::drop_database::test_drop_database_recover
thread 'ddl::tests::drop_database::test_drop_database_recover' panicked at /Users/hyi/prs/greptimedb/src/common/procedure-test/src/lib.rs:137:5:
procedure 'metasrv-procedure::DropDatabase' did not reach the expected state
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

  Cancelling due to test failure: 7 tests still running
        PASS [   0.008s] common-meta key::flow::table_flow::tests::test_key_deserialization
        PASS [   0.008s] common-meta key::flow::table_flow::tests::test_key_serialization
        PASS [   0.008s] common-meta key::flow::tests::test_create_flow_metadata_flow_exists_err
        PASS [   0.010s] common-meta key::flow::tests::test_create_flow_metadata
        PASS [   0.009s] common-meta key::flow::tests::test_create_flow_metadata_unexpected_err
        PASS [   0.531s] common-greptimedb-telemetry tests::test_gretimedb_telemetry
        PASS [   1.962s] cmd cli::tests::test_export_create_table_with_quoted_names
────────────
     Summary [   2.140s] 411/3137 tests run: 410 passed, 1 failed, 8 skipped
  TRY 4 FAIL [   0.019s] common-meta ddl::tests::drop_database::test_drop_database_recover
warning: 2726/3137 tests were not run due to test failure (run with --no-fail-fast to run all tests, or run with --max-fail)

yihong0618 · 2025-02-21T13:12:56Z

still think old way is better and maybe for the recover, so holding this.

check in the drop logical region side do not break any test
to ignore the logical table seems left some empty dir in the data(which I will confirm)
and easy to understand, and also for the later change for the code

WenyXu · 2025-02-24T02:47:26Z

address by use else path way. and frankly speaking, I prefer the previous implementation in the drop logical function seems no particular issue for that?

If all logical tables are force dropped via drop physical region request, we don't need to send drop logical table requests to datanode. By the way, we don't need to handle the (DropTableTarget::Logical, TableRouteValue::Logical(route)) branch anymore; just ignore it.

greptimedb/src/common/meta/src/ddl/drop_database/cursor.rs

Lines 64 to 91 in f6f617d

async fn handle_table(

&mut self,

ddl_ctx: &DdlContext,

ctx: &mut DropDatabaseContext,

table_name: String,

table_id: TableId,

table_route_value: TableRouteValue,

) -> Result<(Box<dyn State>, Status)> {

match (self.target, table_route_value) {

(DropTableTarget::Logical, TableRouteValue::Logical(route)) => {

let physical_table_id = route.physical_table_id();

let (_, table_route) = ddl_ctx

.table_metadata_manager

.table_route_manager()

.get_physical_table_route(physical_table_id)

.await?;

Ok((

Box::new(DropDatabaseExecutor::new(

table_id,

table_id,

TableName::new(&ctx.catalog, &ctx.schema, &table_name),

table_route.region_routes,

self.target,

)),

Status::executing(true),

))

}

using this way do not check is logical region can be recover? the test_drop_database_recover also failed @WenyXu

error local

running 1 test
2025-02-21T13:04:34.395303Z  INFO common_telemetry::logging: logs dir = /tmp/__unittest_logs
2025-02-21T13:04:34.395877Z DEBUG common_meta::ddl::table_meta: Allocated region wal options {0: "{\"wal.provider\":\"raft_engine\"}"} for table 1024
2025-02-21T13:04:34.396312Z  WARN common_meta::ddl::create_logical_tables::update_metadata: No physical columns found, leaving the physical table's schema unchanged when creating logical tables
2025-02-21T13:04:34.396379Z  INFO common_meta::ddl::create_logical_tables::update_metadata: Created 1 tables [1025] metadata for physical table 1024
2025-02-21T13:04:34.397300Z DEBUG common_meta::ddl::drop_table::executor: Dropping region 4398046511104(1024, 0) on Datanode Peer { id: 0, addr: "" }
2025-02-21T13:04:34.397335Z DEBUG common_meta::ddl::test_util::datanode_handler: Returning Ok(0) for request: RegionRequest { header: Some(RegionRequestHeader { tracing_context: {}, dbname: "", query_context: None }), body: Some(Drop(DropRequest { region_id: 4398046511104, force_drop_all_logical_tables: true })) }, peer: Peer { id: 0, addr: "" }
2025-02-21T13:04:34.397358Z  INFO common_meta::ddl::drop_database::executor: Table: greptime.public.phy(1024) is dropped
test ddl::tests::drop_database::test_drop_database_recover ... FAILED

failures:

failures:
    ddl::tests::drop_database::test_drop_database_recover

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 248 filtered out; finished in 0.01s

──── TRY 3 STDERR:       common-meta ddl::tests::drop_database::test_drop_database_recover
thread 'ddl::tests::drop_database::test_drop_database_recover' panicked at /Users/hyi/prs/greptimedb/src/common/procedure-test/src/lib.rs:137:5:
procedure 'metasrv-procedure::DropDatabase' did not reach the expected state
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

   RETRY 4/4 [         ] common-meta ddl::tests::drop_database::test_drop_database_recover
        PASS [   0.006s] common-meta key::flow::flow_info::tests::test_key_serialization
        PASS [   0.007s] common-meta key::flow::flow_name::tests::test_key_serialization
        PASS [   0.011s] common-meta key::flow::flow_info::tests::test_key_deserialization
        PASS [   0.008s] common-meta key::flow::flow_name::tests::test_key_start_range
        PASS [   0.007s] common-meta key::flow::flow_route::tests::test_key_deserialization
        PASS [   0.014s] common-meta key::flow::flow_name::tests::test_key_deserialization
        PASS [   0.008s] common-meta key::flow::flow_route::tests::test_key_serialization
        PASS [   0.009s] common-meta key::flow::flow_route::tests::test_key_start_range
        PASS [   0.008s] common-meta key::flow::flownode_flow::tests::test_key_serialization
        PASS [   0.009s] common-meta key::flow::flownode_flow::tests::test_key_deserialization
  TRY 4 FAIL [   0.019s] common-meta ddl::tests::drop_database::test_drop_database_recover
──── TRY 4 STDOUT:       common-meta ddl::tests::drop_database::test_drop_database_recover

running 1 test
2025-02-21T13:04:34.415082Z  INFO common_telemetry::logging: logs dir = /tmp/__unittest_logs
2025-02-21T13:04:34.415863Z DEBUG common_meta::ddl::table_meta: Allocated region wal options {0: "{\"wal.provider\":\"raft_engine\"}"} for table 1024
2025-02-21T13:04:34.416323Z  WARN common_meta::ddl::create_logical_tables::update_metadata: No physical columns found, leaving the physical table's schema unchanged when creating logical tables
2025-02-21T13:04:34.416394Z  INFO common_meta::ddl::create_logical_tables::update_metadata: Created 1 tables [1025] metadata for physical table 1024
2025-02-21T13:04:34.417372Z DEBUG common_meta::ddl::drop_table::executor: Dropping region 4398046511104(1024, 0) on Datanode Peer { id: 0, addr: "" }
test ddl::tests::drop_database::test_drop_database_recover ... FAILED

failures:

failures:
    ddl::tests::drop_database::test_drop_database_recover

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 248 filtered out; finished in 0.01s

──── TRY 4 STDERR:       common-meta ddl::tests::drop_database::test_drop_database_recover
thread 'ddl::tests::drop_database::test_drop_database_recover' panicked at /Users/hyi/prs/greptimedb/src/common/procedure-test/src/lib.rs:137:5:
procedure 'metasrv-procedure::DropDatabase' did not reach the expected state
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

  Cancelling due to test failure: 7 tests still running
        PASS [   0.008s] common-meta key::flow::table_flow::tests::test_key_deserialization
        PASS [   0.008s] common-meta key::flow::table_flow::tests::test_key_serialization
        PASS [   0.008s] common-meta key::flow::tests::test_create_flow_metadata_flow_exists_err
        PASS [   0.010s] common-meta key::flow::tests::test_create_flow_metadata
        PASS [   0.009s] common-meta key::flow::tests::test_create_flow_metadata_unexpected_err
        PASS [   0.531s] common-greptimedb-telemetry tests::test_gretimedb_telemetry
        PASS [   1.962s] cmd cli::tests::test_export_create_table_with_quoted_names
────────────
     Summary [   2.140s] 411/3137 tests run: 410 passed, 1 failed, 8 skipped
  TRY 4 FAIL [   0.019s] common-meta ddl::tests::drop_database::test_drop_database_recover
warning: 2726/3137 tests were not run due to test failure (run with --no-fail-fast to run all tests, or run with --max-fail)

Yes, you can update the unit test.

WenyXu · 2025-02-24T02:49:00Z

check in the drop logical region side do not break any test

We can just update the test.

to ignore the logical table seems left some empty dir in the data(which I will confirm)

It wouldn't.

and easy to understand, and also for the later change for the code

I think left some comments are aceeptable.

yihong0618 · 2025-02-24T02:51:07Z

and easy to unders

copy that and thank you very much for the confirm

WenyXu · 2025-02-24T02:54:03Z

check in the drop logical region side do not break any test

BTW, I’ll help check the test when I have some free time. It shouldn’t be too tricky.

yihong0618 · 2025-02-24T02:55:32Z

check in the drop logical region side do not break any test

BTW, I’ll help check the test when I have some free time. It shouldn’t be too tricky.

Thanks again learned that!

src/common/meta/src/ddl/drop_database/cursor.rs

src/common/meta/src/ddl/tests/drop_database.rs

src/mito2/src/request.rs

WenyXu · 2025-02-24T13:38:50Z

I realize the changes in this PR might be a bit tricky. However, this is a critical feature that I’d like to include in the 0.12 release. Would you mind if I make the adjustments directly and push them to this PR? Feel free to share your thoughts.

yihong0618 · 2025-02-24T13:41:36Z

I realize the changes in this PR might be a bit tricky. However, this is a critical feature that I’d like to include in the 0.12 release. Would you mind if I make the adjustments directly and push them to this PR? Feel free to share your thoughts.

just continue commit if you have some thought I missed, on this Pull request is nice I wonder, And I'd like to learn from you, thanks again

and I will fix the review comments

WenyXu · 2025-02-24T13:54:00Z

I realize the changes in this PR might be a bit tricky. However, this is a critical feature that I’d like to include in the 0.12 release. Would you mind if I make the adjustments directly and push them to this PR? Feel free to share your thoughts.

just continue commit if you have some thought I missed, on this Pull request is nice I wonder, And I'd like to learn from you, thanks again

and I will fix the review comments

Great, I’ve added some detailed comments. Feel free to reach out if you have any questions.

Signed-off-by: yihong0618 <[email protected]>

yihong0618 · 2025-02-25T07:02:40Z

rebased for easy change

yihong0618 · 2025-02-25T07:58:21Z

I realize the changes in this PR might be a bit tricky. However, this is a critical feature that I’d like to include in the 0.12 release. Would you mind if I make the adjustments directly and push them to this PR? Feel free to share your thoughts.

just continue commit if you have some thought I missed, on this Pull request is nice I wonder, And I'd like to learn from you, thanks again
and I will fix the review comments

Great, I’ve added some detailed comments. Feel free to reach out if you have any questions.

address and add co-author
tests maybe check tonight

WenyXu · 2025-02-25T08:09:32Z

Hi, I realized this PR could be a little tricky. Would you be okay if I made changes directly on your code base? If we can't avoid sending RPCs to the datanode, fast_path would be more appropriate than force_drop_all_logical_tables.

yihong0618 · 2025-02-25T08:10:34Z

Hi, I realized this PR could be a little tricky. Would you be okay if I made changes directly on your code base? If we can't avoid sending RPCs to the datanode, fast_path would be more appropriate than force_drop_all_logical_tables.

of course and I would like that..

yihong0618 · 2025-02-25T08:11:14Z

and FYI:

using your way is sure had empty dir
can you use my script to run them times?

WenyXu · 2025-02-25T08:26:54Z

using your way is sure had empty dir

It's expected.

yihong0618 requested a review from waynexia as a code owner February 18, 2025 08:09

github-actions bot added the docs-not-required This change does not impact docs. label Feb 18, 2025

fengjiachun reviewed Feb 18, 2025

View reviewed changes

src/metric-engine/src/metadata_region.rs Show resolved Hide resolved

yihong0618 force-pushed the hy/fix_issue_4974 branch from 97b5140 to 051a1ac Compare February 19, 2025 13:50

yihong0618 requested review from a team, MichaelScofield, evenyag and v0y4g3r as code owners February 19, 2025 13:50

yihong0618 mentioned this pull request Feb 19, 2025

fix: add fast_drop_path for droprequest GreptimeTeam/greptime-proto#218

Merged

2 tasks

fengjiachun approved these changes Feb 20, 2025

View reviewed changes

fengjiachun reviewed Feb 20, 2025

View reviewed changes

src/store-api/src/region_request.rs Outdated Show resolved Hide resolved

WenyXu reviewed Feb 20, 2025

View reviewed changes

WenyXu reviewed Feb 24, 2025

View reviewed changes

WenyXu mentioned this pull request Feb 24, 2025

Release v0.12 #5509

Closed

26 tasks

perf: do not delete columns when drop logical region in drop database

e442a72

Signed-off-by: yihong0618 <[email protected]>

yihong0618 added 4 commits February 25, 2025 15:01

fix: make ci happy

5b97c87

Signed-off-by: yihong0618 <[email protected]>

fix: address review comments

7174548

Signed-off-by: yihong0618 <[email protected]>

fix: address some comments

13b18c0

Signed-off-by: yihong0618 <[email protected]>

fix: drop stupid comments by copilot

96deb58

Signed-off-by: yihong0618 <[email protected]>

yihong0618 force-pushed the hy/fix_issue_4974 branch from 7c33468 to 83f4071 Compare February 25, 2025 07:01

yihong0618 force-pushed the hy/fix_issue_4974 branch from 83f4071 to b23f6c0 Compare February 25, 2025 07:57

chore: minor refactor

54cf8cc

WenyXu force-pushed the hy/fix_issue_4974 branch from 0dace20 to 8042c69 Compare February 25, 2025 08:32

WenyXu added 2 commits February 25, 2025 08:34

chore: minor refactor

2cfc893

chore: update grpetime-proto

f5b3de8

WenyXu force-pushed the hy/fix_issue_4974 branch from 8042c69 to f5b3de8 Compare February 25, 2025 08:41

WenyXu approved these changes Feb 25, 2025

View reviewed changes

WenyXu enabled auto-merge February 25, 2025 08:51

WenyXu added this pull request to the merge queue Feb 25, 2025

Merged via the queue into GreptimeTeam:main with commit ff0dcf1 Feb 25, 2025
37 checks passed

yihong0618 mentioned this pull request Jul 22, 2025

从给 Greptime 贡献代码的过程中，我都学到了什么 yihong0618/gitblog#320

Open

perf: close issue 4974 by do not delete columns when drop logical region about 100 times faster #5561

perf: close issue 4974 by do not delete columns when drop logical region about 100 times faster #5561

Uh oh!

Conversation

yihong0618 commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

PR Checklist

Uh oh!

coderabbitai bot commented Feb 18, 2025

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

Uh oh!

yihong0618 commented Feb 19, 2025

Uh oh!

fengjiachun left a comment

Choose a reason for hiding this comment

Uh oh!

fengjiachun commented Feb 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yihong0618 commented Feb 21, 2025

Uh oh!

WenyXu commented Feb 21, 2025

Uh oh!

yihong0618 commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yihong0618 commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WenyXu commented Feb 24, 2025

Uh oh!

WenyXu commented Feb 24, 2025

Uh oh!

yihong0618 commented Feb 24, 2025

Uh oh!

WenyXu commented Feb 24, 2025

Uh oh!

yihong0618 commented Feb 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WenyXu commented Feb 24, 2025

Uh oh!

yihong0618 commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WenyXu commented Feb 24, 2025

Uh oh!

yihong0618 commented Feb 25, 2025

Uh oh!

yihong0618 commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WenyXu commented Feb 25, 2025

Uh oh!

yihong0618 commented Feb 25, 2025

Uh oh!

yihong0618 commented Feb 25, 2025

Uh oh!

WenyXu commented Feb 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

yihong0618 commented Feb 18, 2025 •

edited

Loading

yihong0618 commented Feb 21, 2025 •

edited

Loading

yihong0618 commented Feb 21, 2025 •

edited

Loading

yihong0618 commented Feb 24, 2025 •

edited

Loading

yihong0618 commented Feb 25, 2025 •

edited

Loading