Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[Transaction] Fix concurrent modification exception when broker close #1493

Conversation

Demogorgon314
Copy link
Member

Motivation

There is some flaky test when we stop the brokers.

Error:  Failures: 
Error:    CacheInvalidatorTest.cleanup:111->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
Error:    TransactionWithOAuthBearerAuthTest.cleanup:72->TransactionTest.cleanup:77->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer

The root cause is when we close the TransactionMarkerChannelManager,
we will traverse all entries of handlerMap,
but some of the entry already been removed,
we can use ConcurrentHashMap to avoid it.

Modifications

Use ConcurrentHashMap instead of HashMap.

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@Demogorgon314 Demogorgon314 self-assigned this Sep 15, 2022
@github-actions github-actions bot added the no-need-doc This pr does not need any document label Sep 15, 2022
@Demogorgon314 Demogorgon314 force-pushed the Demogorgon314/Fix-ConcurrentModificationException branch from 6085032 to a7bb789 Compare September 15, 2022 08:56
@Demogorgon314 Demogorgon314 marked this pull request as ready for review September 15, 2022 09:12
@BewareMyPower BewareMyPower merged commit 533efbd into streamnative:master Sep 15, 2022
@Demogorgon314 Demogorgon314 deleted the Demogorgon314/Fix-ConcurrentModificationException branch September 15, 2022 09:45
Demogorgon314 added a commit that referenced this pull request Sep 15, 2022
### Motivation

There is some flaky test when we stop the brokers.
```
Error:  Failures:
Error:    CacheInvalidatorTest.cleanup:111->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
Error:    TransactionWithOAuthBearerAuthTest.cleanup:72->TransactionTest.cleanup:77->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
```

The root cause is when we close the `TransactionMarkerChannelManager`,
we will traverse all entries of `handlerMap`,
but some of the entry already been removed,
we can use `ConcurrentHashMap` to avoid it.

### Modifications

Use `ConcurrentHashMap` instead of `HashMap`.

(cherry picked from commit 533efbd)
BewareMyPower pushed a commit that referenced this pull request Sep 15, 2022
### Motivation

There is some flaky test when we stop the brokers.
```
Error:  Failures:
Error:    CacheInvalidatorTest.cleanup:111->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
Error:    TransactionWithOAuthBearerAuthTest.cleanup:72->TransactionTest.cleanup:77->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
```

The root cause is when we close the `TransactionMarkerChannelManager`,
we will traverse all entries of `handlerMap`,
but some of the entry already been removed,
we can use `ConcurrentHashMap` to avoid it.

### Modifications

Use `ConcurrentHashMap` instead of `HashMap`.

(cherry picked from commit 533efbd)
Demogorgon314 added a commit that referenced this pull request Sep 20, 2022
### Motivation

There is some flaky test when we stop the brokers.
```
Error:  Failures:
Error:    CacheInvalidatorTest.cleanup:111->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
Error:    TransactionWithOAuthBearerAuthTest.cleanup:72->TransactionTest.cleanup:77->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
```

The root cause is when we close the `TransactionMarkerChannelManager`,
we will traverse all entries of `handlerMap`,
but some of the entry already been removed,
we can use `ConcurrentHashMap` to avoid it.

### Modifications

Use `ConcurrentHashMap` instead of `HashMap`.

(cherry picked from commit 533efbd)
Demogorgon314 added a commit that referenced this pull request Sep 28, 2022
### Motivation

There is some flaky test when we stop the brokers.
```
Error:  Failures:
Error:    CacheInvalidatorTest.cleanup:111->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
Error:    TransactionWithOAuthBearerAuthTest.cleanup:72->TransactionTest.cleanup:77->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
```

The root cause is when we close the `TransactionMarkerChannelManager`,
we will traverse all entries of `handlerMap`,
but some of the entry already been removed,
we can use `ConcurrentHashMap` to avoid it.

### Modifications

Use `ConcurrentHashMap` instead of `HashMap`.

(cherry picked from commit 533efbd)
michaeljmarshall pushed a commit to michaeljmarshall/kop that referenced this pull request Dec 13, 2022
### Motivation

There is some flaky test when we stop the brokers.
```
Error:  Failures:
Error:    CacheInvalidatorTest.cleanup:111->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
Error:    TransactionWithOAuthBearerAuthTest.cleanup:72->TransactionTest.cleanup:77->KopProtocolHandlerTestBase.internalCleanup:319->KopProtocolHandlerTestBase.stopBroker:355->KopProtocolHandlerTestBase.stopBroker:351 » PulsarServer
```

The root cause is when we close the `TransactionMarkerChannelManager`,
we will traverse all entries of `handlerMap`,
but some of the entry already been removed,
we can use `ConcurrentHashMap` to avoid it.

### Modifications

Use `ConcurrentHashMap` instead of `HashMap`.

(cherry picked from commit 533efbd)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants