You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/10/06 21:55:41 UTC
[GitHub] [trafficserver] bneradt opened a new pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
bneradt opened a new pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243
A performance issue was noticed in Docs testing related to the
RolledLogDeleter candidates consideration. This fixes the candidate
consideration logic to not sort on consideration of every candidate but
rather sort after all the candidates have been gathered (if deletion
will indeed take place).
This addresses issue #7242.
----------------------------------------------------------------
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
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243#discussion_r500621576
##########
File path: proxy/logging/RolledLogDeleter.h
##########
@@ -201,14 +201,27 @@ class RolledLogDeleter
*/
void clear_candidates();
+private:
+ /** Sort all the assembled candidates for each LogDeletingInfo.
+ *
+ * After any additions to the @a deleting_info, this should be called before
+ * calling @a take_next_candidate_to_delete because the latter depends upon
+ * the candidate entries being sorted.
+ */
+ void sort_candidates();
+
private:
/** The owning references to the set of LogDeletingInfo added to the below
* hash map. */
- std::list<std::unique_ptr<LogDeletingInfo>> deletingInfoList;
+ std::deque<std::unique_ptr<LogDeletingInfo>> deletingInfoList;
/** The set of candidates for deletion keyed by log_type. */
IntrusiveHashMap<LogDeletingInfoDescriptor> deleting_info;
/** The number of tracked candidates. */
size_t num_candidates = 0;
+
+ /** Whether the candidates have been sorted since the last addition to the
Review comment:
Isn't that backwards? If the candidates have been sorted since the last addition, why would `candidates_require_sorting` be true?
----------------------------------------------------------------
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
[GitHub] [trafficserver] bneradt commented on pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243#issuecomment-705736462
This closes issue #7242.
----------------------------------------------------------------
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
[GitHub] [trafficserver] bneradt merged pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
Posted by GitBox <gi...@apache.org>.
bneradt merged pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243
----------------------------------------------------------------
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
[GitHub] [trafficserver] zwoop commented on pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243#issuecomment-705734041
Cherry-picked to v9.0.x branch.
----------------------------------------------------------------
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
[GitHub] [trafficserver] bneradt commented on pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243#issuecomment-705736462
This closes issue #7242.
----------------------------------------------------------------
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
[GitHub] [trafficserver] zwoop commented on pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243#issuecomment-705734041
Cherry-picked to v9.0.x branch.
----------------------------------------------------------------
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
[GitHub] [trafficserver] bneradt commented on a change in pull request #7243: RolledLogDeleter: do not sort on each candidate consideration.
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7243:
URL: https://github.com/apache/trafficserver/pull/7243#discussion_r500625590
##########
File path: proxy/logging/RolledLogDeleter.h
##########
@@ -201,14 +201,27 @@ class RolledLogDeleter
*/
void clear_candidates();
+private:
+ /** Sort all the assembled candidates for each LogDeletingInfo.
+ *
+ * After any additions to the @a deleting_info, this should be called before
+ * calling @a take_next_candidate_to_delete because the latter depends upon
+ * the candidate entries being sorted.
+ */
+ void sort_candidates();
+
private:
/** The owning references to the set of LogDeletingInfo added to the below
* hash map. */
- std::list<std::unique_ptr<LogDeletingInfo>> deletingInfoList;
+ std::deque<std::unique_ptr<LogDeletingInfo>> deletingInfoList;
/** The set of candidates for deletion keyed by log_type. */
IntrusiveHashMap<LogDeletingInfoDescriptor> deleting_info;
/** The number of tracked candidates. */
size_t num_candidates = 0;
+
+ /** Whether the candidates have been sorted since the last addition to the
Review comment:
True. I'll update the comment.
----------------------------------------------------------------
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