[
https://issues.apache.org/jira/browse/ARROW-1829?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16256355#comment-16256355
]
ASF GitHub Bot commented on ARROW-1829:
---------------------------------------
pcmoritz closed pull request #1317: ARROW-1829: [Plasma] Fixes to eviction
policy.
URL: https://github.com/apache/arrow/pull/1317
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/cpp/src/plasma/eviction_policy.cc
b/cpp/src/plasma/eviction_policy.cc
index 6c2309f17..a7758fd2c 100644
--- a/cpp/src/plasma/eviction_policy.cc
+++ b/cpp/src/plasma/eviction_policy.cc
@@ -61,38 +61,32 @@ int64_t EvictionPolicy::choose_objects_to_evict(int64_t
num_bytes_required,
}
/* Update the number of bytes used. */
memory_used_ -= bytes_evicted;
+ ARROW_CHECK(memory_used_ >= 0);
return bytes_evicted;
}
void EvictionPolicy::object_created(const ObjectID& object_id) {
auto entry = store_info_->objects[object_id].get();
cache_.add(object_id, entry->info.data_size + entry->info.metadata_size);
+ int64_t size = entry->info.data_size + entry->info.metadata_size;
+ memory_used_ += size;
+ ARROW_CHECK(memory_used_ <= store_info_->memory_capacity);
}
bool EvictionPolicy::require_space(int64_t size,
std::vector<ObjectID>* objects_to_evict) {
/* Check if there is enough space to create the object. */
int64_t required_space = memory_used_ + size - store_info_->memory_capacity;
- int64_t num_bytes_evicted;
- if (required_space > 0) {
- /* Try to free up at least as much space as we need right now but ideally
- * up to 20% of the total capacity. */
- int64_t space_to_free = std::max(size, store_info_->memory_capacity / 5);
- ARROW_LOG(DEBUG) << "not enough space to create this object, so evicting
objects";
- /* Choose some objects to evict, and update the return pointers. */
- num_bytes_evicted = choose_objects_to_evict(space_to_free,
objects_to_evict);
- ARROW_LOG(INFO) << "There is not enough space to create this object, so
evicting "
- << objects_to_evict->size() << " objects to free up "
- << num_bytes_evicted << " bytes.";
- } else {
- num_bytes_evicted = 0;
- }
- if (num_bytes_evicted >= required_space) {
- /* We only increment the space used if there is enough space to create the
- * object. */
- memory_used_ += size;
- }
- return num_bytes_evicted >= required_space;
+ /* Try to free up at least as much space as we need right now but ideally
+ * up to 20% of the total capacity. */
+ int64_t space_to_free = std::max(required_space,
store_info_->memory_capacity / 5);
+ ARROW_LOG(DEBUG) << "not enough space to create this object, so evicting
objects";
+ /* Choose some objects to evict, and update the return pointers. */
+ int64_t num_bytes_evicted = choose_objects_to_evict(space_to_free,
objects_to_evict);
+ ARROW_LOG(INFO) << "There is not enough space to create this object, so
evicting "
+ << objects_to_evict->size() << " objects to free up "
+ << num_bytes_evicted << " bytes.";
+ return num_bytes_evicted >= required_space && num_bytes_evicted > 0;
}
void EvictionPolicy::begin_object_access(const ObjectID& object_id,
diff --git a/cpp/src/plasma/eviction_policy.h b/cpp/src/plasma/eviction_policy.h
index de33dabcb..cebf35b1c 100644
--- a/cpp/src/plasma/eviction_policy.h
+++ b/cpp/src/plasma/eviction_policy.h
@@ -73,8 +73,7 @@ class EvictionPolicy {
void object_created(const ObjectID& object_id);
/// This method will be called when the Plasma store needs more space,
perhaps
- /// to create a new object. If the required amount of space cannot be freed
up,
- /// then a fatal error will be thrown. When this method is called, the
eviction
+ /// to create a new object. When this method is called, the eviction
/// policy will assume that the objects chosen to be evicted will in fact be
/// evicted from the Plasma store by the caller.
///
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> [Plasma] Clean up eviction policy bookkeeping
> ---------------------------------------------
>
> Key: ARROW-1829
> URL: https://issues.apache.org/jira/browse/ARROW-1829
> Project: Apache Arrow
> Issue Type: Bug
> Components: Plasma (C++)
> Reporter: Robert Nishihara
> Assignee: Robert Nishihara
> Labels: pull-request-available
> Fix For: 0.8.0
>
>
> Currently, the eviction policy has a field {{memory_used_}} which keeps track
> of how much memory the store is currently using. However, this field is only
> updated when {{require_space}} is called, and it should be updated every time
> an object is created.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
