Skip to content

Conversation

@jollidah
Copy link
Contributor

@jollidah jollidah commented Aug 31, 2024

  1. Simplify duplicated situation(leaving leader) with for loop
  2. Develop test conditions:
    • Before code change:
      1. Validate leader_id
      2. Validate cluster size
    • After code change:
      1. Validate leader_id
      2. Validate cluster size
      3. Validate follower’s term
      4. Track raw node's StateRole change

@jollidah jollidah marked this pull request as ready for review August 31, 2024 06:23
@jopemachine jopemachine changed the title refact: simplify duplicated simulation and develop test condition refactor: Simplify duplicated simulation and improve test condition Aug 31, 2024
});
}
hash_set.len()
}
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why the counting needs to be executed multiple times for 1 second, rather than just once after a 1-second delay?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use a loop because the tick interval is unpredictable, so repeatedly checking within the 2-second window ensures we capture all relevant state changes.

Copy link
Member

@jopemachine jopemachine Sep 11, 2024

Choose a reason for hiding this comment

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

tick_interval is fixed.
If you mean to election_tick, we can fix it using min_election_tick and max_election_tick.

Copy link
Contributor Author

@jollidah jollidah Sep 12, 2024

Choose a reason for hiding this comment

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

Currently, tick_interval is fixed in this test. We need to modify it to handle varying tick_interval values

@jollidah jollidah force-pushed the refact/test-leader-election branch 2 times, most recently from 6aa4c5f to 9fdfa85 Compare September 10, 2024 01:58
.expect("Failed to execute kill command");
}

pub async fn collect_state_matching_rafts_until_leader_emerge(
Copy link
Member

Choose a reason for hiding this comment

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

I think collect_state_matching_rafts_until_leader_emerge is verbose and performs too many tasks.
I think it would be better to separate the functionality of returning rafts that match a specific state and waiting for a leader to emerge.

@jopemachine jopemachine force-pushed the refact/test-leader-election branch from ef10415 to 64c0934 Compare September 11, 2024 06:55
@jopemachine jopemachine added refactoring Rewrite something in better way while keeping the same functionality test labels Sep 11, 2024
@jollidah jollidah force-pushed the refact/test-leader-election branch 2 times, most recently from 8abf1c8 to 926b404 Compare September 15, 2024 06:41
@jollidah jollidah force-pushed the refact/test-leader-election branch from 95280e1 to d466abb Compare October 2, 2024 08:57
@jopemachine jopemachine force-pushed the refact/test-leader-election branch from fc5f7d3 to 99df4b1 Compare October 4, 2024 06:56
@jopemachine jopemachine merged commit 641578e into lablup:main Oct 4, 2024
@jopemachine jopemachine added the ossca-24 OSS Contribution Academy mentee's contributions. label Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ossca-24 OSS Contribution Academy mentee's contributions. refactoring Rewrite something in better way while keeping the same functionality test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants