Skip to content

[Consensus] Fix concurrent access to IoTConsensus configuration#18290

Open
jt2594838 wants to merge 1 commit into
apache:masterfrom
jt2594838:fix/iot-consensus-concurrent-configuration
Open

[Consensus] Fix concurrent access to IoTConsensus configuration#18290
jt2594838 wants to merge 1 commit into
apache:masterfrom
jt2594838:fix/iot-consensus-concurrent-configuration

Conversation

@jt2594838

Copy link
Copy Markdown
Contributor

Description

Concurrent configuration storage

  • Replace the mutable TreeSet used by IoTConsensusServerImpl with a concurrent set backed by ConcurrentHashMap.newKeySet().
  • Accept the initial peer collection through Collection<Peer> and copy it into the internal concurrent set so callers cannot supply a non-thread-safe implementation.

Deterministic configuration snapshots

  • Sort the copied peer list in getConfiguration() to preserve the deterministic iteration order previously provided by TreeSet.

Concurrency coverage

  • Add a unit test with four concurrent writers and four concurrent readers.
  • Writers add and remove distinct peers while readers continuously obtain configuration snapshots.
  • Verify that snapshots stay duplicate-free and sorted, and that the final membership is exact.

Verification

  • mvn test -pl iotdb-core/consensus -Dtest=IoTConsensusServerImplTest,ReplicateTest,StabilityTest
  • 7 tests passed; Checkstyle and Spotless passed.

This PR has:

  • been self-reviewed.
    • concurrent read
    • concurrent write
    • concurrent read and write
  • added comments explaining the test scenario and expected result.
  • added unit tests to cover the concurrent access path.

Key changed/added classes
  • IoTConsensusServerImpl
  • IoTConsensus
  • IoTConsensusServerImplTest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants