lordgamez commented on code in PR #1519:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1519#discussion_r1124218077
##########
libminifi/src/core/ContentRepository.cpp:
##########
@@ -60,16 +60,29 @@ void ContentRepository::incrementStreamCount(const
minifi::ResourceClaim &stream
}
}
+void ContentRepository::removeFromPurgeList() {
+ for (auto it = purge_list_.begin(); it != purge_list_.end();) {
+ if (removeKey(*it)) {
+ purge_list_.erase(it++);
+ } else {
+ ++it;
+ }
+ }
+}
+
ContentRepository::StreamState ContentRepository::decrementStreamCount(const
minifi::ResourceClaim &streamId) {
std::lock_guard<std::mutex> lock(count_map_mutex_);
+ removeFromPurgeList();
Review Comment:
You are right it shouldn't be the responsibility of `decrementStreamCount`,
I moved it to be part of the `remove` in
bde565c405e3e9419020445a48d41da561b58729 I'm not sure if there should be a
separate thread for this, what do you think?
##########
libminifi/src/core/ContentRepository.cpp:
##########
@@ -60,16 +60,29 @@ void ContentRepository::incrementStreamCount(const
minifi::ResourceClaim &stream
}
}
+void ContentRepository::removeFromPurgeList() {
+ for (auto it = purge_list_.begin(); it != purge_list_.end();) {
+ if (removeKey(*it)) {
+ purge_list_.erase(it++);
+ } else {
+ ++it;
+ }
+ }
+}
+
ContentRepository::StreamState ContentRepository::decrementStreamCount(const
minifi::ResourceClaim &streamId) {
std::lock_guard<std::mutex> lock(count_map_mutex_);
+ removeFromPurgeList();
const std::string str = streamId.getContentFullPath();
auto count = count_map_.find(str);
if (count != count_map_.end() && count->second > 1) {
count_map_[str] = count->second - 1;
return StreamState::Alive;
} else {
count_map_.erase(str);
- remove(streamId);
+ if (!remove(streamId)) {
+ purge_list_.push_back(streamId.getContentFullPath());
+ }
Review Comment:
Updated in bde565c405e3e9419020445a48d41da561b58729
##########
libminifi/include/core/ContentRepository.h:
##########
@@ -53,10 +54,18 @@ class ContentRepository : public
StreamManager<minifi::ResourceClaim>, public ut
virtual void clearOrphans() = 0;
+ bool remove(const minifi::ResourceClaim &streamId) override {
Review Comment:
Updated in bde565c405e3e9419020445a48d41da561b58729
##########
libminifi/include/core/ContentRepository.h:
##########
@@ -53,10 +54,18 @@ class ContentRepository : public
StreamManager<minifi::ResourceClaim>, public ut
virtual void clearOrphans() = 0;
+ bool remove(const minifi::ResourceClaim &streamId) override {
+ return removeKey(streamId.getContentFullPath());
+ }
+
protected:
+ void removeFromPurgeList();
+ virtual bool removeKey(const std::string& content_path) = 0;
+
std::string directory_;
std::mutex count_map_mutex_;
Review Comment:
I prefer having a separate mutex for the purge_list and separating the
`count_map_` and `purge_list_` processing blocks, updated in
bde565c405e3e9419020445a48d41da561b58729
##########
libminifi/src/core/ContentRepository.cpp:
##########
@@ -60,16 +60,29 @@ void ContentRepository::incrementStreamCount(const
minifi::ResourceClaim &stream
}
}
+void ContentRepository::removeFromPurgeList() {
+ for (auto it = purge_list_.begin(); it != purge_list_.end();) {
+ if (removeKey(*it)) {
+ purge_list_.erase(it++);
+ } else {
+ ++it;
+ }
+ }
+}
Review Comment:
Added purge list update in `clearOprhans` in
bde565c405e3e9419020445a48d41da561b58729
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
