This is an automated email from the ASF dual-hosted git repository.
jvanderzee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 0e562cbe17 Remove Stripe::add_agg_todo (#10827)
0e562cbe17 is described below
commit 0e562cbe175f03f29c1dc1172969c2952d8eacb7
Author: JosiahWI <[email protected]>
AuthorDate: Thu Jan 11 17:05:12 2024 -0600
Remove Stripe::add_agg_todo (#10827)
* Reorder handleWrite logic
This is in preparation for extracting logic from the method.
* Extract Stripe::add_writer
* Clean up and get rid of Stripe::add_agg_todo
---
src/iocore/cache/CacheWrite.cc | 20 ++++----------------
src/iocore/cache/P_CacheVol.h | 25 ++++++++++++++++++-------
src/iocore/cache/Stripe.cc | 27 +++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 23 deletions(-)
diff --git a/src/iocore/cache/CacheWrite.cc b/src/iocore/cache/CacheWrite.cc
index 069c0c690c..a52080982b 100644
--- a/src/iocore/cache/CacheWrite.cc
+++ b/src/iocore/cache/CacheWrite.cc
@@ -247,34 +247,22 @@ CacheVC::handleWrite(int event, Event * /* e ATS_UNUSED
*/)
set_agg_write_in_progress();
POP_HANDLER;
- agg_len = stripe->round_to_approx_size(write_len + header_len + frag_len +
sizeof(Doc));
- stripe->add_agg_todo(agg_len);
- bool agg_error = (agg_len > AGG_SIZE || header_len + sizeof(Doc) >
MAX_FRAG_SIZE ||
- (!f.readers && (stripe->get_agg_todo_size() >
cache_config_agg_write_backlog + AGG_SIZE) && write_len));
-#ifdef CACHE_AGG_FAIL_RATE
- agg_error = agg_error ||
((uint32_t)mutex->thread_holding->generator.random() < (uint32_t)(UINT_MAX *
CACHE_AGG_FAIL_RATE));
-#endif
+
bool max_doc_error = (cache_config_max_doc_size &&
(cache_config_max_doc_size < vio.ndone ||
(vio.nbytes != INT64_MAX
&& (cache_config_max_doc_size < vio.nbytes))));
-
- if (agg_error || max_doc_error) {
+ // Make sure the size is correct for checking error conditions before
calling add_writer(this).
+ agg_len = stripe->round_to_approx_size(write_len + header_len + frag_len +
sizeof(Doc));
+ if (max_doc_error || !stripe->add_writer(this)) {
Metrics::Counter::increment(cache_rsb.write_backlog_failure);
Metrics::Counter::increment(stripe->cache_vol->vol_rsb.write_backlog_failure);
Metrics::Counter::increment(cache_rsb.status[op_type].failure);
Metrics::Counter::increment(stripe->cache_vol->vol_rsb.status[op_type].failure);
- stripe->add_agg_todo(-agg_len);
io.aio_result = AIO_SOFT_FAILURE;
if (event == EVENT_CALL) {
return EVENT_RETURN;
}
return handleEvent(AIO_EVENT_DONE, nullptr);
}
- ink_assert(agg_len <= AGG_SIZE);
- if (f.evac_vector) {
- stripe->get_pending_writers().push(this);
- } else {
- stripe->get_pending_writers().enqueue(this);
- }
if (!stripe->is_io_in_progress()) {
return stripe->aggWrite(event, this);
}
diff --git a/src/iocore/cache/P_CacheVol.h b/src/iocore/cache/P_CacheVol.h
index 2f1210464b..3d4d487c27 100644
--- a/src/iocore/cache/P_CacheVol.h
+++ b/src/iocore/cache/P_CacheVol.h
@@ -280,8 +280,25 @@ public:
char *get_agg_buffer();
int get_agg_buf_pos() const;
int get_agg_todo_size() const;
- void add_agg_todo(int size);
+ /**
+ * Add a virtual connection waiting to write to this stripe.
+ *
+ * If vc->f.evac_vector is set, it will be queued before any regular writes.
+ *
+ * This operation may fail for any one of the following reasons:
+ * - The write would overflow the internal aggregation buffer.
+ * - Adding a Doc to the virtual connection header would exceed the
+ * maximum fragment size.
+ * - vc->f.readers is not set (this virtual connection is not an
evacuator),
+ * the internal aggregation buffer is full, the writes waiting to be
+ * aggregated exceed the maximum backlog, and the virtual connection
+ * has a non-zero write length.
+ *
+ * @param vc: The virtual connection.
+ * @return: Returns true if the operation was successfull, otherwise false.
+ */
+ bool add_writer(CacheVC *vc);
bool flush_aggregate_write_buffer();
private:
@@ -585,9 +602,3 @@ Stripe::get_agg_todo_size() const
{
return this->_write_buffer.get_bytes_pending_aggregation();
}
-
-inline void
-Stripe::add_agg_todo(int size)
-{
- this->_write_buffer.add_bytes_pending_aggregation(size);
-}
diff --git a/src/iocore/cache/Stripe.cc b/src/iocore/cache/Stripe.cc
index d445e64965..85ab5da3e2 100644
--- a/src/iocore/cache/Stripe.cc
+++ b/src/iocore/cache/Stripe.cc
@@ -918,6 +918,33 @@ Stripe::_init_data()
this->_init_data_internal();
}
+bool
+Stripe::add_writer(CacheVC *vc)
+{
+ ink_assert(vc);
+ this->_write_buffer.add_bytes_pending_aggregation(vc->agg_len);
+ bool agg_error =
+ (vc->agg_len > AGG_SIZE || vc->header_len + sizeof(Doc) > MAX_FRAG_SIZE ||
+ (!vc->f.readers && (this->_write_buffer.get_bytes_pending_aggregation() >
cache_config_agg_write_backlog + AGG_SIZE) &&
+ vc->write_len));
+#ifdef CACHE_AGG_FAIL_RATE
+ agg_error = agg_error ||
((uint32_t)vc->mutex->thread_holding->generator.random() < (uint32_t)(UINT_MAX
* CACHE_AGG_FAIL_RATE));
+#endif
+
+ if (agg_error) {
+ this->_write_buffer.add_bytes_pending_aggregation(-vc->agg_len);
+ } else {
+ ink_assert(vc->agg_len <= AGG_SIZE);
+ if (vc->f.evac_vector) {
+ this->get_pending_writers().push(vc);
+ } else {
+ this->get_pending_writers().enqueue(vc);
+ }
+ }
+
+ return !agg_error;
+}
+
bool
Stripe::flush_aggregate_write_buffer()
{
