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: switch madsim integration and recovery tests to sql backend #18678

Merged
merged 37 commits into from
Oct 7, 2024

Conversation

yezizp2012
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?

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added test labels as necessary. See details.
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
  • All checks passed in ./risedev check (or alias, ./risedev c)
  • My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)
  • My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

@yezizp2012 yezizp2012 marked this pull request as ready for review September 26, 2024 05:56
@yezizp2012 yezizp2012 requested a review from a team as a code owner September 26, 2024 05:56
@yezizp2012 yezizp2012 requested a review from xxchan September 26, 2024 05:56
@yezizp2012 yezizp2012 changed the title feat: [IGNORE ME]switch madsim integration tests to sql backend feat: switch madsim integration and recovery tests to sql backend Sep 26, 2024
@kwannoel
Copy link
Contributor

Encountered another issue in recovery test. Seems like it occurred when trying to shutdown the simulation cluster. Cc @kwannoel any ideas?

Not sure either. Maybe to do with sqlx. I'm taking a look.

@yezizp2012
Copy link
Member Author

yezizp2012 commented Sep 30, 2024

And this one https://buildkite.com/risingwavelabs/pull-request/builds/59181#0192414d-c986-408c-bf72-81826cef3716:

2022-04-30T13:40:01.912968Z ERROR node{id=5 name="frontend-2"}:task{id=369707}:handle_query{mode="simple query" session_id=0 sql=CREATE MATERIALIZED VIEW mv AS SELECT tm, foo FROM t EMIT ON WINDOW CLOSE}: pgwire::pg_protocol: error when process message error=Failed to run the query: Catalog error: gRPC request to meta service failed: Some entity that we attempted to create already exists: table with name mv exists
2022-04-30T13:40:01.912968Z  INFO node{id=5 name="frontend-2"}:task{id=369707}:handle_query{mode="simple query" session_id=0 sql=CREATE MATERIALIZED VIEW mv AS SELECT tm, foo FROM t EMIT ON WINDOW CLOSE}: pgwire_query_log: status="err" time=31ms
2022-04-30T13:40:01.914625Z  INFO node{id=11 name="client"}:task{id=369708}: tokio_postgres::connection: NOTICE: EMIT ON WINDOW CLOSE is currently an experimental feature. Please use it with caution.    
2022-04-30T13:40:01.914625Z DEBUG node{id=11 name="client"}:task{id=863}: risingwave_simulation::slt: Record Statement { loc: Location { file: "e2e_test/streaming/eowc/eowc_select.slt", line: 17, upper: None }, conditions: [], connection: Default, sql: "create materialized view mv as\nselect tm, foo from t\nemit on window close;", expected: Ok } finished in 102.300447ms
2022-04-30T13:40:02.060582Z  INFO node{id=9 name="compactor-1"}:task{id=368300}: risingwave_storage::hummock::compactor: running_parallelism_count=0 pull_task_ack=false pending_pull_task_count=3
2022-04-30T13:40:02.276794Z  INFO epoch{otel.name="Epoch 2234174752423936" epoch=2234174752423936}: rw_tracing: new barrier enqueued epoch=2234174817959936
2022-04-30T13:40:02.279992Z  WARN node{id=7 name="compute-2"}:task{id=369263}: risingwave_stream::task::barrier_manager: control stream reset with error error=gRPC request failed: Internal error: failed to handle barrier event: Actor 8 exited unexpectedly: actor 3 not found in info table
2022-04-30T13:40:02.279992Z  WARN node{id=7 name="compute-2"}:task{id=369263}: risingwave_stream::task::barrier_manager: actor error overwritten actor_id=7 prev_err=Actor 7 exited unexpectedly: actor 3 not found in info table
2022-04-30T13:40:02.281031Z  WARN node{id=3 name="meta-1"}:task{id=367601}: risingwave_meta::barrier::rpc: get error from response stream node=WorkerNode { id: 3, r#type: ComputeNode, host: Some(HostAddress { host: "192.168.3.2", port: 5688 }), state: Running, property: Some(Property { is_streaming: true, is_serving: true, is_unschedulable: false, internal_rpc_host_addr: "" }), transactional_id: Some(2), resource: Some(Resource { rw_version: "2.1.0-alpha", total_memory_bytes: 66673201152, total_cpu_cores: 2 }), started_at: Some(1651325983), parallelism: 2, node_label: "" } err=gRPC request to stream service failed: Internal error: failed to handle barrier event: Actor 8 exited unexpectedly: actor 3 not found in info table
2022-04-30T13:40:02.281031Z  INFO node{id=3 name="meta-1"}:task{id=367601}: risingwave_telemetry_event: Telemetry tracking_id is not set, event reporting disabled
2022-04-30T13:40:02.281031Z  INFO node{id=3 name="meta-1"}:task{id=367601}:failure_recovery{error=get error from control stream, in worker node 3: gRPC request to stream service failed: Internal error: failed to handle barrier event: Actor 8 exited unexpectedly: actor 3 not found in info table}: risingwave_meta::barrier::recovery: recovery start!
2022-04-30T13:40:02.281031Z DEBUG node{id=3 name="meta-1"}:task{id=369582}: risingwave_meta::stream::stream_manager: stream job failed id=TableId { table_id: 977 }
2022-04-30T13:40:02.281031Z ERROR node{id=3 name="meta-1"}:task{id=369582}: risingwave_meta::rpc::ddl_controller_v2: failed to create streaming job id=977 error=get error from control stream, in worker node 3: gRPC request to stream service failed: Internal error: failed to handle barrier event: Actor 8 exited unexpectedly: actor 3 not found in info table
2022-04-30T13:40:02.281035Z  WARN node{id=3 name="meta-1"}:task{id=369582}: risingwave_meta::rpc::ddl_controller_v2: aborted streaming job id=977

...

2022-04-30T13:40:16.120591Z  INFO node{id=5 name="frontend-2"}:task{id=369707}:handle_query{mode="simple query" session_id=0 sql=SELECT * FROM mv ORDER BY tm}: pgwire_query_log: status="err" time=0ms
2022-04-30T13:40:16.126742Z DEBUG node{id=11 name="client"}:task{id=863}: risingwave_simulation::slt: Record Query { loc: Location { file: "e2e_test/streaming/eowc/eowc_select.slt", line: 31, upper: None }, conditions: [], connection: Default, sql: "select * from mv order by tm;", expected: Results { types: [Text, Integer], sort_mode: None, label: None, results: ["2023-05-06 16:51:00  1", "2023-05-06 16:56:00  8", "2023-05-06 17:30:00  3"] } } finished in 14.281116ms
thread '<unnamed>' panicked at /risingwave/src/tests/simulation/src/slt.rs:321:29:
query failed: db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
  1: Catalog error
  2: table or source not found: mv

[SQL] select * from mv order by tm;

Actually mv was failed to create and got Catalog error: gRPC request to meta service failed: Some entity that we attempted to create already exists: table with name mv exists error after retry. But the streaming job was aborted after it. The simulation test treated it as finished in the previous one:

| SqlCmd::CreateMaterializedView { .. }
if i != 0
&& e.to_string().contains("exists")
&& e.to_string().contains("Catalog error") =>

We can fix it by changing the match pattern of checking error messages.

@kwannoel kwannoel force-pushed the feat/switch-sim-integration-tests branch 3 times, most recently from 963f285 to 537c8d8 Compare September 30, 2024 11:29
@kwannoel kwannoel force-pushed the feat/switch-sim-integration-tests branch from e4efa0c to c227f11 Compare September 30, 2024 12:06
@kwannoel

This comment was marked as duplicate.

@kwannoel
Copy link
Contributor

kwannoel commented Oct 1, 2024

Passed 2 rounds of ci. @xxchan @BugenZhao can help to take a look?

I'd like to get this merged, and work on subsequent parts incrementally. We can always revert if something goes terribly wrong.

@kwannoel
Copy link
Contributor

kwannoel commented Oct 1, 2024

Meta backup test failed: https://buildkite.com/risingwavelabs/main-cron/builds/3448#01924664-243c-4e6c-a9f6-a9efcc92a0b8

failed to run `e2e_test/backup_restore/tpch_snapshot_create.slt`
Caused by:
    statement failed: db error: ERROR: Failed to run the query
    Caused by these errors (recent errors listed first):
      1: gRPC request to meta service failed: Internal error
      2: Hummock error
      3: SST 2 is rejected from being committed since it's below watermark: SST timestamp 1727758344, meta node timestamp 1727758345, retention_sec 0, watermark 1727758345

Edit: Nevermind, it already failed in today's main-cron.

@xxchan
Copy link
Member

xxchan commented Oct 1, 2024

I'd like to get this merged, and work on subsequent parts incrementally.

I'd be happy to proceed in this way

Copy link
Member

@xxchan xxchan left a comment

Choose a reason for hiding this comment

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

Quickly browsed the changes. I don't think it can do anything too harmful. So rubber stamp 😁

Comment on lines +11 to +13
[meta.developer]
meta_actor_cnt_per_worker_parallelism_soft_limit = 65536
meta_actor_cnt_per_worker_parallelism_hard_limit = 65536
Copy link
Member

Choose a reason for hiding this comment

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

How's this related with etcd vs sql? 👀

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me add this to the list of things to follow up on.

DbBackend::Sqlite => {
Arc::new(SqlBackendElectionClient::new(id, SqliteDriver::new(conn)))
}
DbBackend::Sqlite => Arc::new(DummyElectionClient::new(id)),
Copy link
Member

Choose a reason for hiding this comment

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

Should we just disallow multiple meta when it's SQLite..? (I'm fine leaving it as UB..)

@kwannoel
Copy link
Contributor

kwannoel commented Oct 7, 2024

To summarize, here's what's needed after this PR is merged:

@kwannoel kwannoel added this pull request to the merge queue Oct 7, 2024
Merged via the queue into main with commit ba761f2 Oct 7, 2024
30 of 32 checks passed
@kwannoel kwannoel deleted the feat/switch-sim-integration-tests branch October 7, 2024 01:20
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.

4 participants