You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/12/24 05:41:30 UTC
[GitHub] [pulsar-client-go] dferstay opened a new pull request #689: Provide lock-free access to partition consumer compression providers
dferstay opened a new pull request #689:
URL: https://github.com/apache/pulsar-client-go/pull/689
### Motivation
The compression providers map in the partition consumer is a textbook case
for using go's lock-free sync.Map: the set of map entries is stable and
access is read-only.
On machines with 4 cores or greater, read contention on the sync.RWMutex
outweighs the cost of using a sync.Map.
Below is an old article on the subject, but it still holds true today:
https://medium.com/@deckarep/the-new-kid-in-town-gos-sync-map-de24a6bf7c2c
### Modifications
The sync.RWMutex and map of compression providers (`map[pb.CompressionType]compression.Provider`) in the partition consumer has been replaced with a sync.Map
### Verifying this change
This change is already covered by existing tests, such as:
- `pulsar/consumer_partitiion_test.go`
### Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): No
- The public API: No
- The schema: No
- The default values of configurations: No
- The wire protocol: No
### Documentation
- Does this pull request introduce a new feature? No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] dferstay commented on a change in pull request #689: [Partition Consumer] Provide lock-free access to compression providers
Posted by GitBox <gi...@apache.org>.
dferstay commented on a change in pull request #689:
URL: https://github.com/apache/pulsar-client-go/pull/689#discussion_r775107621
##########
File path: pulsar/consumer_partition.go
##########
@@ -1190,19 +1193,26 @@ func getPreviousMessage(mid trackingMessageID) trackingMessageID {
}
func (pc *partitionConsumer) Decompress(msgMeta *pb.MessageMetadata, payload internal.Buffer) (internal.Buffer, error) {
- pc.providersMutex.RLock()
- provider, ok := pc.compressionProviders[msgMeta.GetCompression()]
- pc.providersMutex.RUnlock()
+ v, ok := pc.compressionProviders.Load(msgMeta.GetCompression())
Review comment:
@wolfstudy , I've renamed the variables in this method for additional clarity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on a change in pull request #689: [Partition Consumer] Provide lock-free access to compression providers
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on a change in pull request #689:
URL: https://github.com/apache/pulsar-client-go/pull/689#discussion_r774949519
##########
File path: pulsar/consumer_partition.go
##########
@@ -1190,19 +1193,26 @@ func getPreviousMessage(mid trackingMessageID) trackingMessageID {
}
func (pc *partitionConsumer) Decompress(msgMeta *pb.MessageMetadata, payload internal.Buffer) (internal.Buffer, error) {
- pc.providersMutex.RLock()
- provider, ok := pc.compressionProviders[msgMeta.GetCompression()]
- pc.providersMutex.RUnlock()
+ v, ok := pc.compressionProviders.Load(msgMeta.GetCompression())
Review comment:
Maybe we can replace `v` with meaningful variable names
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy merged pull request #689: [Partition Consumer] Provide lock-free access to compression providers
Posted by GitBox <gi...@apache.org>.
wolfstudy merged pull request #689:
URL: https://github.com/apache/pulsar-client-go/pull/689
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] wolfstudy commented on a change in pull request #689: [Partition Consumer] Provide lock-free access to compression providers
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on a change in pull request #689:
URL: https://github.com/apache/pulsar-client-go/pull/689#discussion_r775190280
##########
File path: pulsar/consumer_partition.go
##########
@@ -1190,19 +1193,26 @@ func getPreviousMessage(mid trackingMessageID) trackingMessageID {
}
func (pc *partitionConsumer) Decompress(msgMeta *pb.MessageMetadata, payload internal.Buffer) (internal.Buffer, error) {
- pc.providersMutex.RLock()
- provider, ok := pc.compressionProviders[msgMeta.GetCompression()]
- pc.providersMutex.RUnlock()
+ v, ok := pc.compressionProviders.Load(msgMeta.GetCompression())
Review comment:
Cool, nice work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] dferstay commented on a change in pull request #689: [Partition Consumer] Provide lock-free access to compression providers
Posted by GitBox <gi...@apache.org>.
dferstay commented on a change in pull request #689:
URL: https://github.com/apache/pulsar-client-go/pull/689#discussion_r775107320
##########
File path: pulsar/consumer_partition.go
##########
@@ -1190,19 +1193,26 @@ func getPreviousMessage(mid trackingMessageID) trackingMessageID {
}
func (pc *partitionConsumer) Decompress(msgMeta *pb.MessageMetadata, payload internal.Buffer) (internal.Buffer, error) {
- pc.providersMutex.RLock()
- provider, ok := pc.compressionProviders[msgMeta.GetCompression()]
- pc.providersMutex.RUnlock()
+ v, ok := pc.compressionProviders.Load(msgMeta.GetCompression())
Review comment:
Sure thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org