You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2024/12/11 19:41:51 UTC
(kudu) branch branch-1.18.x updated (412e32eff -> a969b2848)
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a change to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
from 412e32eff KUDU-613: Remove use of vectors under lock
new 4967eb0ae KUDU-613: Specialize template Insert functions
new a969b2848 [jwt] do not leak temporary test file
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/kudu/util/jwt-util-test.cc | 37 ++++++++++++++++++++++-----------
src/kudu/util/slru_cache.cc | 46 +++++++++++++++++++++++-------------------
src/kudu/util/slru_cache.h | 14 +++++--------
3 files changed, 55 insertions(+), 42 deletions(-)
(kudu) 01/02: KUDU-613: Specialize template Insert functions
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 4967eb0ae5ca6153a2a158a4e369ba9f9d6f1e8a
Author: Mahesh Reddy <mr...@cloudera.com>
AuthorDate: Tue Dec 10 15:35:22 2024 -0500
KUDU-613: Specialize template Insert functions
This patch refactors the Insert adjacent functions to
use template specialization. Now only two functions exist,
Insert() and ReInsert().
This patch does not contain any functional modifications.
Change-Id: I39d45fa96bb0a758d059a43ccc0dff47b0c3f472
Reviewed-on: http://gerrit.cloudera.org:8080/22193
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Alexey Serbin <al...@apache.org>
(cherry picked from commit 0259a4dbbb19e72bbe7554cfc924d391bf886622)
Reviewed-on: http://gerrit.cloudera.org:8080/22196
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
Tested-by: Abhishek Chennaka <ac...@cloudera.com>
---
src/kudu/util/slru_cache.cc | 46 ++++++++++++++++++++++++---------------------
src/kudu/util/slru_cache.h | 14 +++++---------
2 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/src/kudu/util/slru_cache.cc b/src/kudu/util/slru_cache.cc
index 751d95404..0c97e8b61 100644
--- a/src/kudu/util/slru_cache.cc
+++ b/src/kudu/util/slru_cache.cc
@@ -248,6 +248,7 @@ void SLRUCacheShard<Segment::kProbationary>::RemoveEntriesPastCapacity() {
}
}
+// Inserts handle into the probationary shard and removes any entries past capacity.
template<>
Handle* SLRUCacheShard<Segment::kProbationary>::Insert(SLRUHandle* handle,
EvictionCallback* eviction_callback) {
@@ -277,26 +278,10 @@ Handle* SLRUCacheShard<Segment::kProbationary>::Insert(SLRUHandle* handle,
return reinterpret_cast<Handle*>(handle);
}
+// Inserts handle into the protected shard when updating entry in the protected segment.
template<>
-void SLRUCacheShard<Segment::kProtected>::UpgradeInsert(SLRUHandle* handle) {
- handle->in_protected_segment.store(true, std::memory_order_relaxed);
- if (PREDICT_TRUE(metrics_)) {
- metrics_->upgrades->Increment();
- metrics_->protected_segment_cache_usage->IncrementBy(handle->charge);
- metrics_->protected_segment_inserts->Increment();
- }
-
- handle->Sanitize();
- RL_Append(handle);
-
- // No entries should exist with same key in protected segment when upgrading.
- SLRUHandle* old_entry = table_.Insert(handle);
- DCHECK(old_entry == nullptr);
-}
-
-template<>
-Handle* SLRUCacheShard<Segment::kProtected>::UpdateInsert(SLRUHandle* handle,
- EvictionCallback* eviction_callback) {
+Handle* SLRUCacheShard<Segment::kProtected>::Insert(SLRUHandle* handle,
+ EvictionCallback* eviction_callback) {
handle->eviction_callback = eviction_callback;
// Two refs for the handle: one from SLRUCacheShard, one for the returned handle.
// Even though this function is for updates in the protected segment, it's treated similarly
@@ -324,6 +309,7 @@ Handle* SLRUCacheShard<Segment::kProtected>::UpdateInsert(SLRUHandle* handle,
return reinterpret_cast<Handle*>(handle);
}
+// Inserts handle into the probationary shard when downgrading entry from the protected segment.
template<>
void SLRUCacheShard<Segment::kProbationary>::ReInsert(SLRUHandle* handle) {
handle->in_protected_segment.store(false, std::memory_order_relaxed);
@@ -341,6 +327,24 @@ void SLRUCacheShard<Segment::kProbationary>::ReInsert(SLRUHandle* handle) {
RemoveEntriesPastCapacity();
}
+// Inserts handle into the protected shard when upgrading entry from the probationary segment.
+template<>
+void SLRUCacheShard<Segment::kProtected>::ReInsert(SLRUHandle* handle) {
+ handle->in_protected_segment.store(true, std::memory_order_relaxed);
+ if (PREDICT_TRUE(metrics_)) {
+ metrics_->upgrades->Increment();
+ metrics_->protected_segment_cache_usage->IncrementBy(handle->charge);
+ metrics_->protected_segment_inserts->Increment();
+ }
+
+ handle->Sanitize();
+ RL_Append(handle);
+
+ // No entries should exist with same key in protected segment when upgrading.
+ SLRUHandle* old_entry = table_.Insert(handle);
+ DCHECK(old_entry == nullptr);
+}
+
template<Segment segment>
void SLRUCacheShard<segment>::Erase(const Slice& key, uint32_t hash) {
SLRUHandle* e = table_.Remove(key, hash);
@@ -393,7 +397,7 @@ Handle* SLRUCacheShardPair::Insert(SLRUHandle* handle,
if (!ProtectedContains(handle->key(), handle->hash)) {
return probationary_shard_.Insert(handle, eviction_callback);
}
- auto* inserted = protected_shard_.UpdateInsert(handle, eviction_callback);
+ auto* inserted = protected_shard_.Insert(handle, eviction_callback);
// If newly inserted entry has greater charge than previous one,
// possible that entries can be evicted if at capacity.
DowngradeEntries();
@@ -444,7 +448,7 @@ Handle* SLRUCacheShardPair::Lookup(const Slice& key, uint32_t hash, bool caching
// probationary segment then adding entry to the protected segment.
// Downgrade any entries in the protected segment if past capacity.
probationary_shard_.SoftErase(key, hash);
- protected_shard_.UpgradeInsert(val_handle);
+ protected_shard_.ReInsert(val_handle);
DowngradeEntries();
return probationary_handle;
diff --git a/src/kudu/util/slru_cache.h b/src/kudu/util/slru_cache.h
index 6d8613868..2cee6b0da 100644
--- a/src/kudu/util/slru_cache.h
+++ b/src/kudu/util/slru_cache.h
@@ -115,14 +115,12 @@ class SLRUCacheShard {
void SetMetrics(SLRUCacheMetrics* metrics) { metrics_ = metrics; }
- // Inserts handle into the probationary shard and removes any entries if past capacity.
- // Returns the inserted handle.
+ // Inserts handle into the appropriate shard and returns the inserted handle.
+ // See comments on template specialization for each function for more details.
Handle* Insert(SLRUHandle* handle, EvictionCallback* eviction_callback);
- // Inserts handle into the protected shard when upgrading entry from the probationary segment.
- void UpgradeInsert(SLRUHandle* handle);
- // Inserts handle into the protected shard when updating entry in protected segment.
- // Returns the inserted handle.
- Handle* UpdateInsert(SLRUHandle* handle, EvictionCallback* eviction_callback);
+ // Inserts handle into the appropriate shard when upgrading or downgrading entry.
+ // See comments on template specialization for each function for more details.
+ void ReInsert(SLRUHandle* handle);
// Like SLRUCache::Lookup, but with an extra "hash" parameter.
Handle* Lookup(const Slice& key, uint32_t hash, bool caching);
// Reduces the entry's ref by one, frees the entry if no refs are remaining.
@@ -134,8 +132,6 @@ class SLRUCacheShard {
void SoftErase(const Slice& key, uint32_t hash);
// Returns true if shard contains entry, false if not.
bool Contains(const Slice& key, uint32_t hash);
- // Inserts handle into the probationary shard when downgrading entry from the protected segment.
- void ReInsert(SLRUHandle* handle);
// Update the high-level metrics for a lookup operation.
void UpdateMetricsLookup(bool was_hit, bool caching);
// Update the segment-level metrics for a lookup operation.
(kudu) 02/02: [jwt] do not leak temporary test file
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit a969b28482d9672b189150259483795f0697d89d
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Dec 10 15:27:54 2024 -0800
[jwt] do not leak temporary test file
Prior to this patch, many temporary files would be left under /tmp
directory after running jwt-util-test. This patch addresses the issue.
Change-Id: I52481670ba4e20a1b33f777eebf8d3fe618f873f
Reviewed-on: http://gerrit.cloudera.org:8080/22194
Tested-by: Kudu Jenkins
Reviewed-by: Zoltan Chovan <zc...@cloudera.com>
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
(cherry picked from commit e1e7079e7541559c34742cb311ba806f35efbd54)
Reviewed-on: http://gerrit.cloudera.org:8080/22199
Tested-by: Alexey Serbin <al...@apache.org>
---
src/kudu/util/jwt-util-test.cc | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/src/kudu/util/jwt-util-test.cc b/src/kudu/util/jwt-util-test.cc
index a6e1cf341..c5dd93fff 100644
--- a/src/kudu/util/jwt-util-test.cc
+++ b/src/kudu/util/jwt-util-test.cc
@@ -62,10 +62,11 @@ using kudu::MiniOidc;
/// Utility class for creating a file that will be automatically deleted upon test
/// completion.
-class TempTestDataFile {
+class TempTestDataFile final {
public:
// Creates a temporary file with the specified contents.
explicit TempTestDataFile(const std::string& contents);
+ ~TempTestDataFile();
/// Returns the absolute path to the file.
const std::string& Filename() const { return name_; }
@@ -75,23 +76,35 @@ class TempTestDataFile {
std::unique_ptr<WritableFile> tmp_file_;
};
-TempTestDataFile::TempTestDataFile(const std::string& contents)
- : name_("/tmp/jwks_XXXXXX") {
- string created_filename;
+TempTestDataFile::TempTestDataFile(const std::string& contents) {
+ static const string cNamePattern = "/tmp/jwks_XXXXXX";
WritableFileOptions opts;
opts.is_sensitive = false;
- Status status;
- status = Env::Default()->NewTempWritableFile(opts, &name_[0], &created_filename, &tmp_file_);
- if (!status.ok()) {
- std::cerr << Substitute("Error creating temp file: $0", created_filename);
+ string fname;
+ if (auto s = Env::Default()->NewTempWritableFile(
+ opts, cNamePattern, &fname, &tmp_file_); !s.ok()) {
+ std::cerr << Substitute("error creating temp file '$0': $1",
+ fname, s.ToString());
}
- status = WriteStringToFile(Env::Default(), contents, created_filename);
- if (!status.ok()) {
- std::cerr << Substitute("Error writing contents to temp file: $0", created_filename);
+ if (auto s = WriteStringToFile(Env::Default(), contents, fname);
+ !s.ok()) {
+ std::cerr << Substitute("error writing into temp file '$0': $1",
+ fname, s.ToString());
}
- name_ = created_filename;
+ name_ = fname;
+}
+
+TempTestDataFile::~TempTestDataFile() {
+ if (tmp_file_) {
+ // Remove the temporary file from the filesystem. The destructor
+ // of the 'tmp_file_' field takes care of closing the file.
+ if (auto s = Env::Default()->DeleteFile(name_); !s.ok()) {
+ std::cerr << Substitute("error removing temp file '$0': $1",
+ name_, s.ToString());
+ }
+ }
}
TEST(JwtUtilTest, LoadJwksFile) {