You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2020/11/07 15:44:52 UTC
[GitHub] [incubator-pegasus] Smityz opened a new pull request #640: feat(hotkey): add thread_safe method to clear collector
Smityz opened a new pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a change in pull request #640: feat(hotkey): add thread_safe method to clear collector
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640#discussion_r519523051
##########
File path: src/server/hotkey_collector.cpp
##########
@@ -332,5 +339,12 @@ void hotkey_fine_data_collector::analyse_data(detect_hotkey_result &result)
}
}
+void hotkey_fine_data_collector::clear()
+{
+ std::pair<dsn::blob, uint64_t> key_weight_pair;
+ while (_capture_key_queue.try_dequeue(key_weight_pair)) {
+ };
Review comment:
```suggestion
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] Smityz commented on a change in pull request #640: feat(hotkey): add thread_safe method to clear collector
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640#discussion_r519543723
##########
File path: src/server/hotkey_collector.cpp
##########
@@ -332,5 +339,12 @@ void hotkey_fine_data_collector::analyse_data(detect_hotkey_result &result)
}
}
+void hotkey_fine_data_collector::clear()
+{
+ std::pair<dsn::blob, uint64_t> key_weight_pair;
Review comment:
```
concurrentqueue.h:890
// Swaps this queue's state with the other's. Not thread-safe.
// Swapping two queues does not invalidate their tokens, however
// the tokens that were created for one queue must be used with
// only the swapped queue (i.e. the tokens are tied to the
// queue's movable state, not the object itself).
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] Smityz commented on a change in pull request #640: feat(hotkey): add thread_safe method to clear collector
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640#discussion_r519543723
##########
File path: src/server/hotkey_collector.cpp
##########
@@ -332,5 +339,12 @@ void hotkey_fine_data_collector::analyse_data(detect_hotkey_result &result)
}
}
+void hotkey_fine_data_collector::clear()
+{
+ std::pair<dsn::blob, uint64_t> key_weight_pair;
Review comment:
```
concurrentqueue.h:890
// Swaps this queue's state with the other's. Not thread-safe.
// Swapping two queues does not invalidate their tokens, however
// the tokens that were created for one queue must be used with
// only the swapped queue (i.e. the tokens are tied to the
// queue's movable state, not the object itself).
```
It's not a thread safe way
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a change in pull request #640: feat(hotkey): add thread_safe method to clear collector
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640#discussion_r519899763
##########
File path: src/server/hotkey_collector.h
##########
@@ -102,6 +102,7 @@ class internal_collector_base : public dsn::replication::replica_base
explicit internal_collector_base(replica_base *base) : replica_base(base){};
virtual void capture_data(const dsn::blob &hash_key, uint64_t weight) = 0;
virtual void analyse_data(detect_hotkey_result &result) = 0;
+ virtual void clear() = 0;
Review comment:
Where you call this function?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] Smityz commented on a change in pull request #640: feat(hotkey): add thread_safe method to clear collector
Posted by GitBox <gi...@apache.org>.
Smityz commented on a change in pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640#discussion_r520242847
##########
File path: src/server/hotkey_collector.h
##########
@@ -102,6 +102,7 @@ class internal_collector_base : public dsn::replication::replica_base
explicit internal_collector_base(replica_base *base) : replica_base(base){};
virtual void capture_data(const dsn::blob &hash_key, uint64_t weight) = 0;
virtual void analyse_data(detect_hotkey_result &result) = 0;
+ virtual void clear() = 0;
Review comment:
https://github.com/apache/incubator-pegasus/pull/632/files#diff-7d14c5784befda2ae449c643a1ec2dd2b9988db5fc72325f48e855b6a566e02eR126
In another PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] neverchanje merged pull request #640: feat(hotkey): add thread_safe method to clear collector
Posted by GitBox <gi...@apache.org>.
neverchanje merged pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] levy5307 commented on a change in pull request #640: feat(hotkey): add thread_safe method to clear collector
Posted by GitBox <gi...@apache.org>.
levy5307 commented on a change in pull request #640:
URL: https://github.com/apache/incubator-pegasus/pull/640#discussion_r519541092
##########
File path: src/server/hotkey_collector.cpp
##########
@@ -332,5 +339,12 @@ void hotkey_fine_data_collector::analyse_data(detect_hotkey_result &result)
}
}
+void hotkey_fine_data_collector::clear()
+{
+ std::pair<dsn::blob, uint64_t> key_weight_pair;
Review comment:
what about change it like this?
```
moodycamel::ConcurrentQueue<std::pair<dsn::blob, uint64_t>> temp;
_capture_key_queue.swap(temp);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org