This is an automated email from the ASF dual-hosted git repository.
bneradt 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 ec659190b6 Obliterate Stripe::reset_agg_buf_pos (#10786)
ec659190b6 is described below
commit ec659190b635404a8a91e62b58f2bf1c4880e72e
Author: JosiahWI <41302989+josia...@users.noreply.github.com>
AuthorDate: Thu Nov 16 17:30:18 2023 -0600
Obliterate Stripe::reset_agg_buf_pos (#10786)
There was only one use of Stripe::reset_agg_buf_pos, and this gets rid of
it by moving the code to flush the aggregate buffer from CacheDir.cc into
Stripe.cc and AggregateWriteBuffer.cc.
I also removed two #defines from P_CacheVol.h that were supposed to be
moved to AggregateWriteBuffer.h, but got left in due to a rebase mistake and
were duplicated in both places.
---
include/iocore/cache/AggregateWriteBuffer.h | 16 ++++++++++++
src/iocore/cache/AggregateWriteBuffer.cc | 40 +++++++++++++++++++++++++++++
src/iocore/cache/CMakeLists.txt | 1 +
src/iocore/cache/CacheDir.cc | 15 +----------
src/iocore/cache/P_CacheVol.h | 11 ++------
src/iocore/cache/Stripe.cc | 18 +++++++++++++
6 files changed, 78 insertions(+), 23 deletions(-)
diff --git a/include/iocore/cache/AggregateWriteBuffer.h
b/include/iocore/cache/AggregateWriteBuffer.h
index 1ead5ba51e..1e0456b204 100644
--- a/include/iocore/cache/AggregateWriteBuffer.h
+++ b/include/iocore/cache/AggregateWriteBuffer.h
@@ -53,6 +53,22 @@ public:
AggregateWriteBuffer(AggregateWriteBuffer &&other) = delete;
AggregateWriteBuffer &operator=(AggregateWriteBuffer &&other) = delete;
+ /**
+ * Flush the internal buffer to disk.
+ *
+ * This method should be called during shutdown. It must not be called
+ * during regular operation.
+ *
+ * Flushing the buffer only writes the buffer to disk; it does not
+ * modify the contents of the buffer. To reset the buffer, call
+ * reset_buffer_pos().
+ *
+ * @param fd File descriptor to write to.
+ * @param write_pos The offset at which to write the buffer data.
+ * @return Returns true if all bytes were flushed, otherwise false.
+ */
+ bool flush(int fd, off_t write_pos) const;
+
Queue<CacheVC, Continuation::Link_link> &get_pending_writers();
char *get_buffer();
int get_buffer_pos() const;
diff --git a/src/iocore/cache/AggregateWriteBuffer.cc
b/src/iocore/cache/AggregateWriteBuffer.cc
new file mode 100644
index 0000000000..52e96c667d
--- /dev/null
+++ b/src/iocore/cache/AggregateWriteBuffer.cc
@@ -0,0 +1,40 @@
+/** @file
+
+ A brief file description
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#include "iocore/cache/AggregateWriteBuffer.h"
+
+#include "iocore/aio/AIO_fault_injection.h"
+
+#include "tscore/ink_assert.h"
+#include "tscore/ink_platform.h"
+
+bool
+AggregateWriteBuffer::flush(int fd, off_t write_pos) const
+{
+ int r = pwrite(fd, this->_buffer, this->_buffer_pos, write_pos);
+ if (r != this->_buffer_pos) {
+ ink_assert(!"flushing agg buffer failed");
+ return false;
+ }
+ return true;
+}
diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt
index 4d9dfc073c..4d7f9498ad 100644
--- a/src/iocore/cache/CMakeLists.txt
+++ b/src/iocore/cache/CMakeLists.txt
@@ -17,6 +17,7 @@
add_library(
inkcache STATIC
+ AggregateWriteBuffer.cc
Cache.cc
CacheDir.cc
CacheDisk.cc
diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc
index 43fe7520d3..a5ff7eddcb 100644
--- a/src/iocore/cache/CacheDir.cc
+++ b/src/iocore/cache/CacheDir.cc
@@ -998,20 +998,7 @@ sync_cache_dir_on_shutdown()
// directories have not been inserted for these writes
if (vol->get_agg_buf_pos()) {
Dbg(dbg_ctl_cache_dir_sync, "Dir %s: flushing agg buffer first",
vol->hash_text.get());
-
- // set write limit
- vol->header->agg_pos = vol->header->write_pos + vol->get_agg_buf_pos();
-
- int r = pwrite(vol->fd, vol->get_agg_buffer(), vol->get_agg_buf_pos(),
vol->header->write_pos);
- if (r != vol->get_agg_buf_pos()) {
- ink_assert(!"flushing agg buffer failed");
- continue;
- }
- vol->header->last_write_pos = vol->header->write_pos;
- vol->header->write_pos += vol->get_agg_buf_pos();
- ink_assert(vol->header->write_pos == vol->header->agg_pos);
- vol->reset_agg_buf_pos();
- vol->header->write_serial++;
+ vol->flush_aggregate_write_buffer();
}
if (buflen < dirlen) {
diff --git a/src/iocore/cache/P_CacheVol.h b/src/iocore/cache/P_CacheVol.h
index 5e2902d1a5..f1b7bfcfc1 100644
--- a/src/iocore/cache/P_CacheVol.h
+++ b/src/iocore/cache/P_CacheVol.h
@@ -43,8 +43,6 @@
#define STRIPE_MAGIC 0xF1D0F00D
#define START_BLOCKS 16 // 8k, STORE_BLOCK_SIZE
#define START_POS ((off_t)START_BLOCKS * CACHE_BLOCK_SIZE)
-#define AGG_SIZE (4 * 1024 * 1024) // 4MB
-#define AGG_HIGH_WATER (AGG_SIZE / 2) // 2MB
#define EVACUATION_SIZE (2 * AGG_SIZE) // 8MB
#define STRIPE_BLOCK_SIZE (1024 * 1024 * 128) // 128MB
#define MIN_STRIPE_SIZE STRIPE_BLOCK_SIZE
@@ -281,10 +279,11 @@ public:
Queue<CacheVC, Continuation::Link_link> &get_pending_writers();
char *get_agg_buffer();
int get_agg_buf_pos() const;
- void reset_agg_buf_pos();
int get_agg_todo_size() const;
void add_agg_todo(int size);
+ bool flush_aggregate_write_buffer();
+
private:
void _clear_init();
void _init_dir();
@@ -579,12 +578,6 @@ Stripe::get_agg_buf_pos() const
return this->_write_buffer.get_buffer_pos();
}
-inline void
-Stripe::reset_agg_buf_pos()
-{
- this->_write_buffer.reset_buffer_pos();
-}
-
inline int
Stripe::get_agg_todo_size() const
{
diff --git a/src/iocore/cache/Stripe.cc b/src/iocore/cache/Stripe.cc
index 818d1480f7..631f8c13c1 100644
--- a/src/iocore/cache/Stripe.cc
+++ b/src/iocore/cache/Stripe.cc
@@ -917,3 +917,21 @@ Stripe::_init_data()
this->_init_data_internal();
this->_init_data_internal();
}
+
+bool
+Stripe::flush_aggregate_write_buffer()
+{
+ // set write limit
+ this->header->agg_pos = this->header->write_pos +
this->_write_buffer.get_buffer_pos();
+
+ if (!this->_write_buffer.flush(this->fd, this->header->write_pos)) {
+ return false;
+ }
+ this->header->last_write_pos = this->header->write_pos;
+ this->header->write_pos += this->_write_buffer.get_buffer_pos();
+ ink_assert(this->header->write_pos == this->header->agg_pos);
+ this->_write_buffer.reset_buffer_pos();
+ this->header->write_serial++;
+
+ return true;
+}
