szaszm commented on code in PR #1328:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1328#discussion_r925832155
##########
libminifi/include/core/repository/VolatileContentRepository.h:
##########
@@ -68,55 +59,55 @@ class VolatileContentRepository :
}
master_list_.clear();
}
+ stop();
}
/**
* Initialize the volatile content repo
* @param configure configuration
*/
- virtual bool initialize(const std::shared_ptr<Configure> &configure);
-
- /**
- * Stop any thread associated with the volatile content repository.
- */
- virtual void stop();
+ bool initialize(const std::shared_ptr<Configure> &configure) override;
/**
* Creates writable stream.
* @param claim resource claim
* @return BaseStream shared pointer that represents the stream the consumer
will write to.
*/
- virtual std::shared_ptr<io::BaseStream> write(const minifi::ResourceClaim
&claim, bool append);
+ std::shared_ptr<io::BaseStream> write(const minifi::ResourceClaim &claim,
bool append) override;
/**
* Creates readable stream.
* @param claim resource claim
* @return BaseStream shared pointer that represents the stream from which
the consumer will read..
*/
- virtual std::shared_ptr<io::BaseStream> read(const minifi::ResourceClaim
&claim);
+ std::shared_ptr<io::BaseStream> read(const minifi::ResourceClaim &claim)
override;
- virtual bool exists(const minifi::ResourceClaim &streamId);
+ bool exists(const minifi::ResourceClaim &streamId) override;
/**
* Closes the claim.
* @return whether or not the claim is associated with content stored in
volatile memory.
*/
- virtual bool close(const minifi::ResourceClaim &claim) {
+ bool close(const minifi::ResourceClaim &claim) override {
return remove(claim);
}
/**
* Closes the claim.
* @return whether or not the claim is associated with content stored in
volatile memory.
*/
- virtual bool remove(const minifi::ResourceClaim &claim);
+ bool remove(const minifi::ResourceClaim &claim) override;
+
+ private:
+ void run() override {
+ }
- protected:
- virtual void start();
+ std::thread& getThread() override {
+ return thread_;
+ }
Review Comment:
This class doesn't need a thread. Can you think of a structure that doesn't
require all repositories to have a thread?
##########
libminifi/include/core/Repository.h:
##########
@@ -218,56 +195,77 @@ class Repository : public virtual
core::SerializableComponent, public core::Trac
return Put(key, buffer, bufferSize);
}
- uint64_t incrementSize(const char* /*fpath*/, const struct stat *sb, int
/*typeflag*/) {
- return (repo_size_ += sb->st_size);
- }
-
virtual void loadComponent(const std::shared_ptr<core::ContentRepository>&
/*content_repo*/) {
}
- virtual uint64_t getRepoSize();
+ virtual uint64_t getRepoSize() const {
+ return repo_size_;
+ }
std::string getDirectory() const {
return directory_;
}
- // Prevent default copy constructor and assignment operation
- // Only support pass by reference or pointer
- Repository(const Repository &parent) = delete;
- Repository &operator=(const Repository &parent) = delete;
+ // Starts repository monitor thread
+ void start() {
+ if (running_.exchange(true)) {
+ return;
+ }
+ if (purge_period_ <= std::chrono::milliseconds(0)) {
+ return;
+ }
+ getThread() = std::thread(&Repository::run, this);
+
+ logger_->log_debug("%s Repository monitor thread start", name_);
+ }
+
+ // Stops repository monitor thread
+ void stop() {
+ if (!running_.exchange(false)) {
+ return;
+ }
+ if (getThread().joinable()) {
+ getThread().join();
+ }
+ logger_->log_debug("%s Repository monitor thread stop", name_);
+ }
Review Comment:
It's possible to start again on another thread after line 224 has finished,
and we're waiting for 227 or 228. One should not be able to start the
repository until stopping has finished. Same with start, you shouldn't be able
to stop until starting has finished.
I suggest using a state enum with 4 values: starting, running, stopping,
stopped.
Only transitions to the right and stopped -> starting should be possible.
--
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]
