Skip to content

Commit

Permalink
fix: correct set_region_role_state_gracefully behaviors (#5171)
Browse files Browse the repository at this point in the history
* fix: reduce default max rows for fuzz testing

* chore: remove Postgres setup from fuzz test workflow

* chore(fuzz): increase resource limits for GreptimeDB cluster

* chore(fuzz): increase resource limits for kafka

* fix: correct `set_region_role_state_gracefully` behaviors

* chore: remove Postgres setup from fuzz test workflow

* chore(fuzz): redue resource limits for GreptimeDB & kafka
  • Loading branch information
WenyXu authored and evenyag committed Dec 20, 2024
1 parent 7aa8c28 commit 43c12b4
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 7 deletions.
2 changes: 2 additions & 0 deletions .github/actions/setup-kafka-cluster/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ runs:
--set controller.replicaCount=${{ inputs.controller-replicas }} \
--set controller.resources.requests.cpu=50m \
--set controller.resources.requests.memory=128Mi \
--set controller.resources.limits.cpu=2000m \
--set controller.resources.limits.memory=2Gi \
--set listeners.controller.protocol=PLAINTEXT \
--set listeners.client.protocol=PLAINTEXT \
--create-namespace \
Expand Down
4 changes: 0 additions & 4 deletions .github/workflows/develop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,6 @@ jobs:
uses: ./.github/actions/setup-kafka-cluster
- name: Setup Etcd cluser
uses: ./.github/actions/setup-etcd-cluster
- name: Setup Postgres cluser
uses: ./.github/actions/setup-postgres-cluster
# Prepares for fuzz tests
- uses: arduino/setup-protoc@v3
with:
Expand Down Expand Up @@ -474,8 +472,6 @@ jobs:
uses: ./.github/actions/setup-kafka-cluster
- name: Setup Etcd cluser
uses: ./.github/actions/setup-etcd-cluster
- name: Setup Postgres cluser
uses: ./.github/actions/setup-postgres-cluster
# Prepares for fuzz tests
- uses: arduino/setup-protoc@v3
with:
Expand Down
8 changes: 7 additions & 1 deletion src/metric-engine/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ impl RegionEngine for MetricEngine {
for x in [
utils::to_metadata_region_id(region_id),
utils::to_data_region_id(region_id),
region_id,
] {
if let Err(e) = self.inner.mito.set_region_role(x, role)
&& e.status_code() != StatusCode::RegionNotFound
Expand All @@ -226,6 +225,13 @@ impl RegionEngine for MetricEngine {
region_id: RegionId,
region_role_state: SettableRegionRoleState,
) -> std::result::Result<SetRegionRoleStateResponse, BoxedError> {
self.inner
.mito
.set_region_role_state_gracefully(
utils::to_metadata_region_id(region_id),
region_role_state,
)
.await?;
self.inner
.mito
.set_region_role_state_gracefully(region_id, region_role_state)
Expand Down
3 changes: 3 additions & 0 deletions src/metric-engine/src/engine/catchup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use common_telemetry::debug;
use snafu::ResultExt;
use store_api::region_engine::RegionEngine;
use store_api::region_request::{AffectedRows, RegionCatchupRequest, RegionRequest};
Expand All @@ -35,6 +36,7 @@ impl MetricEngineInner {
}
let metadata_region_id = utils::to_metadata_region_id(region_id);
// TODO(weny): improve the catchup, we can read the wal entries only once.
debug!("Catchup metadata region {metadata_region_id}");
self.mito
.handle_request(
metadata_region_id,
Expand All @@ -48,6 +50,7 @@ impl MetricEngineInner {
.context(MitoCatchupOperationSnafu)?;

let data_region_id = utils::to_data_region_id(region_id);
debug!("Catchup data region {data_region_id}");
self.mito
.handle_request(
data_region_id,
Expand Down
3 changes: 2 additions & 1 deletion src/mito2/src/worker/handle_catchup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
use std::sync::Arc;

use common_telemetry::info;
use common_telemetry::tracing::warn;
use common_telemetry::{debug, info};
use snafu::ensure;
use store_api::logstore::LogStore;
use store_api::region_engine::RegionRole;
Expand All @@ -40,6 +40,7 @@ impl<S: LogStore> RegionWorkerLoop<S> {
};

if region.is_writable() {
debug!("Region {region_id} is writable, skip catchup");
return Ok(0);
}
// Note: Currently, We protect the split brain by ensuring the mutable table is empty.
Expand Down
2 changes: 1 addition & 1 deletion tests-fuzz/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ macro_rules! make_get_from_env_helper {

make_get_from_env_helper!(GT_FUZZ_INPUT_MAX_ALTER_ACTIONS, 256);
make_get_from_env_helper!(GT_FUZZ_INPUT_MAX_INSERT_ACTIONS, 8);
make_get_from_env_helper!(GT_FUZZ_INPUT_MAX_ROWS, 2048);
make_get_from_env_helper!(GT_FUZZ_INPUT_MAX_ROWS, 512);
make_get_from_env_helper!(GT_FUZZ_INPUT_MAX_TABLES, 64);
make_get_from_env_helper!(GT_FUZZ_INPUT_MAX_COLUMNS, 32);

Expand Down

0 comments on commit 43c12b4

Please sign in to comment.