Skip to content

Commit

Permalink
MB-45756: Tidy EventuallyPersistentEngine memory management
Browse files Browse the repository at this point in the history
Refactor EventuallyPersistentEngine to remove the usage of raw pointers
for members checkpointConfig & workload, replacing them with
std::unique_ptr<> to help simplify the dtor of
EventuallyPersistentEngine.

Change-Id: I1f6aa936699b7db6b80ed4d52c2f4b5d360acf57
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/153256
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
  • Loading branch information
rdemellow committed May 11, 2021
1 parent 467af73 commit 8925739
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
9 changes: 5 additions & 4 deletions engines/ep/src/ep_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,8 @@ cb::engine_errc EventuallyPersistentEngine::initialize(const char* config) {
std::make_unique<EpEngineValueChangeListener>(*this));

auto numShards = configuration.getMaxNumShards();
workload = new WorkLoadPolicy(configuration.getMaxNumWorkers(), numShards);
workload = std::make_unique<WorkLoadPolicy>(
configuration.getMaxNumWorkers(), numShards);

const auto& confResMode = configuration.getConflictResolutionType();
if (!setConflictResolutionMode(confResMode)) {
Expand Down Expand Up @@ -2031,7 +2032,7 @@ cb::engine_errc EventuallyPersistentEngine::initialize(const char* config) {
dcpFlowControlManager_ = std::make_unique<DcpFlowControlManager>(*this);
}

checkpointConfig = new CheckpointConfig(*this);
checkpointConfig = std::make_unique<CheckpointConfig>(*this);
CheckpointConfig::addConfigChangeListener(*this);

kvBucket = makeBucket(configuration);
Expand Down Expand Up @@ -6581,8 +6582,8 @@ EventuallyPersistentEngine::~EventuallyPersistentEngine() {
waitForTasks(tasks);
}
EP_LOG_INFO("~EPEngine: Completed deinitialize.");
delete workload;
delete checkpointConfig;
workload.reset();
checkpointConfig.reset();

// Engine going away, tell ArenaMalloc to unregister
cb::ArenaMalloc::unregisterClient(arena);
Expand Down
4 changes: 2 additions & 2 deletions engines/ep/src/ep_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,7 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
*/
Configuration configuration;
std::unique_ptr<KVBucket> kvBucket;
WorkLoadPolicy *workload;
std::unique_ptr<WorkLoadPolicy> workload;
bucket_priority_t workloadPriority;

// The conflict resolution mode for this bucket (as used by XDCR via
Expand All @@ -1287,7 +1287,7 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {

std::unique_ptr<DcpFlowControlManager> dcpFlowControlManager_;
std::unique_ptr<DcpConnMap> dcpConnMap_;
CheckpointConfig *checkpointConfig;
std::unique_ptr<CheckpointConfig> checkpointConfig;
std::string name;
size_t maxItemSize;
size_t maxItemPrivilegedBytes;
Expand Down
4 changes: 2 additions & 2 deletions engines/ep/tests/mock/mock_synchronous_ep_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ SynchronousEPEngine::SynchronousEPEngine(const cb::ArenaMallocClient& client,
// workload is needed by EPStore's constructor (to construct the
// VBucketMap).
auto shards = configuration.getMaxNumShards();
workload = new WorkLoadPolicy(/*workers*/ 1, shards);
workload = std::make_unique<WorkLoadPolicy>(/*workers*/ 1, shards);

// dcpConnMap_ is needed by EPStore's constructor.
dcpConnMap_ = std::make_unique<MockDcpConnMap>(*this);

// checkpointConfig is needed by CheckpointManager (via EPStore).
checkpointConfig = new CheckpointConfig(*this);
checkpointConfig = std::make_unique<CheckpointConfig>(*this);

// Simplified setup for switching FlowControl on/off
if (configuration.getDcpFlowControlPolicy() == "none") {
Expand Down

0 comments on commit 8925739

Please sign in to comment.