[kudu] branch master updated: [security] handle BIO_new

2023-04-30 Thread alexey
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
 new a7684eacf [security] handle BIO_new
a7684eacf is described below

commit a7684eacf26bf84c415b8bb32228d026b639f7a8
Author: Alexey Serbin 
AuthorDate: Fri Apr 28 16:52:12 2023 -0700

[security] handle BIO_new

This patch adds verification of the result returned by the OpenSSL's
BIO_new() function.  It's a rare case when BIO_new() returns nullptr,
but if it does so, let's handle the results as documented.

Change-Id: I7a18034fde219c181f13d2d4c1ee9acb7e8e1a46
Reviewed-on: http://gerrit.cloudera.org:8080/19819
Tested-by: Kudu Jenkins
Reviewed-by: Yuqi Du 
Reviewed-by: Abhishek Chennaka 
---
 src/kudu/security/cert.cc  |  3 +++
 src/kudu/security/cert.h   |  1 -
 src/kudu/security/crypto.cc|  6 ++
 src/kudu/security/tls_handshake.cc | 10 ++
 src/kudu/util/openssl_util.h   |  5 +
 src/kudu/util/openssl_util_bio.h   |  3 +++
 6 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc
index ff5cf0e71..b84e2bada 100644
--- a/src/kudu/security/cert.cc
+++ b/src/kudu/security/cert.cc
@@ -59,6 +59,7 @@ string X509NameToString(X509_NAME* name) {
   SCOPED_OPENSSL_NO_PENDING_ERRORS;
   CHECK(name);
   auto bio = ssl_make_unique(BIO_new(BIO_s_mem()));
+  OPENSSL_CHECK(bio, "could not create memory BIO");
   OPENSSL_CHECK_OK(X509_NAME_print_ex(bio.get(), name, 0, XN_FLAG_ONELINE));
 
   BUF_MEM* membuf;
@@ -213,7 +214,9 @@ Status Cert::GetServerEndPointChannelBindings(string* 
channel_bindings) const {
   // Create a digest BIO. All data written to the BIO will be sent through the
   // digest (hash) function. The digest BIO requires a null BIO to 
writethrough to.
   auto null_bio = ssl_make_unique(BIO_new(BIO_s_null()));
+  OPENSSL_RET_IF_NULL(null_bio, "could not create null BIO");
   auto md_bio = ssl_make_unique(BIO_new(BIO_f_md()));
+  OPENSSL_RET_IF_NULL(md_bio, "could not create message digest BIO");
   OPENSSL_RET_NOT_OK(BIO_set_md(md_bio.get(), md), "failed to set digest for 
BIO");
   BIO_push(md_bio.get(), null_bio.get());
 
diff --git a/src/kudu/security/cert.h b/src/kudu/security/cert.h
index 55bd44fcd..3a00ef4be 100644
--- a/src/kudu/security/cert.h
+++ b/src/kudu/security/cert.h
@@ -20,7 +20,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
diff --git a/src/kudu/security/crypto.cc b/src/kudu/security/crypto.cc
index be2ebf0f0..ea9fc22f3 100644
--- a/src/kudu/security/crypto.cc
+++ b/src/kudu/security/crypto.cc
@@ -268,14 +268,12 @@ Status PrivateKey::GetPublicKey(PublicKey* public_key) 
const {
 return Status::RuntimeError(GetOpenSSLErrors());
   }
   auto tmp = ssl_make_unique(BIO_new(BIO_s_mem()));
-  CHECK(tmp);
+  OPENSSL_RET_IF_NULL(tmp, "could not create memory BIO");
   // Export public key in DER format into the temporary buffer.
   OPENSSL_RET_NOT_OK(i2d_RSA_PUBKEY_bio(tmp.get(), rsa.get()),
   "error extracting public RSA key");
   // Read the public key into the result placeholder.
-  RETURN_NOT_OK(public_key->FromBIO(tmp.get(), DataFormat::DER));
-
-  return Status::OK();
+  return public_key->FromBIO(tmp.get(), DataFormat::DER);
 }
 
 // Modeled after code in $OPENSSL_ROOT/apps/dgst.c
diff --git a/src/kudu/security/tls_handshake.cc 
b/src/kudu/security/tls_handshake.cc
index f3d2e4503..b28df06ad 100644
--- a/src/kudu/security/tls_handshake.cc
+++ b/src/kudu/security/tls_handshake.cc
@@ -101,15 +101,9 @@ Status TlsHandshake::Init(c_unique_ptr s) {
   }
 
   auto rbio = ssl_make_unique(BIO_new(BIO_s_mem()));
-  if (!rbio) {
-return Status::RuntimeError(
-"failed to create memory-based read BIO", GetOpenSSLErrors());
-  }
+  OPENSSL_RET_IF_NULL(rbio, "failed to create memory read BIO");
   auto wbio = ssl_make_unique(BIO_new(BIO_s_mem()));
-  if (!wbio) {
-return Status::RuntimeError(
-"failed to create memory-based write BIO", GetOpenSSLErrors());
-  }
+  OPENSSL_RET_IF_NULL(wbio, "failed to create memory write BIO");
   ssl_ = std::move(s);
   auto* ssl = ssl_.get();
   SSL_set_bio(ssl, rbio.release(), wbio.release());
diff --git a/src/kudu/util/openssl_util.h b/src/kudu/util/openssl_util.h
index 97daf7121..2115c13a5 100644
--- a/src/kudu/util/openssl_util.h
+++ b/src/kudu/util/openssl_util.h
@@ -56,6 +56,11 @@ typedef struct x509_st X509;
 return Status::RuntimeError((msg), security::GetOpenSSLErrors()); \
   }
 
+#define OPENSSL_CHECK(call, msg) \
+  if ((call) == nullptr) { \
+LOG(FATAL) << #msg ": " << security::GetOpenSSLErrors(); \
+  }
+
 #define OPENSSL_RET_IF_NULL(call, msg) \
   if ((call) == nullptr) { \
 return Status::RuntimeError((msg), security::GetOpenSSLErrors()); \
diff --git a/src/kudu/util/openssl_util_bio.h b/src/kudu/u

[kudu] branch master updated: [tablet] GC ancient, fully deleted rowsets without live row count stats

2023-04-30 Thread laiyingchun
This is an automated email from the ASF dual-hosted git repository.

laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
 new e89dfe9a7 [tablet] GC ancient, fully deleted rowsets without live row 
count stats
e89dfe9a7 is described below

commit e89dfe9a7d615b9cdb45e7222ac4dacb0e0916d3
Author: kedeng 
AuthorDate: Fri Mar 31 11:24:16 2023 +0800

[tablet] GC ancient, fully deleted rowsets without live row count stats

We added a background op to GC ancient, fully deleted rowsets for
KUDU-1625 base on live row count. That patch is very useful, but
does not work for older versions(earlier than 1.10) that do not
support live row count stats. And during the upgrade process from
a lower version to a higher version, live row count feature cannot
be enabled for already existing data.

To resolve this issue on a lower version of the kudu cluster, I submitted
this patch. The main reason is to replace the use of live row count.
However, due to the lack of a more accurate counting method, this patch
may only release part of the storage space for ancient, fully deleted rows.
Therefore, this feature can alleviate the storage space tension of older
versions to a certain extent.

If you need to enable this feature, enable the flag
--enable_gc_deleted_rowsets_without_live_row_count and restart tservers.

There's still room for improvement in this implementation in that, 
currently,
we ignored the delete operation in DMS. I will resolve this issue in a 
follow-up
patch.

I ran this on a real cluster, the storage space of deleted rowsets that was 
not
previously freed can be GCed as expected. And I also add unit test case to 
ensure
it make sense.

Change-Id: Iacdff107b8b07cbd56f47f296a93f4bcfbf56b41
Reviewed-on: http://gerrit.cloudera.org:8080/19670
Tested-by: Kudu Jenkins
Reviewed-by: Yingchun Lai 
Reviewed-by: Yuqi Du 
---
 src/kudu/tablet/delta_tracker.cc  | 14 +++
 src/kudu/tablet/delta_tracker.h   |  5 +++
 src/kudu/tablet/diskrowset-test-base.h|  2 +-
 src/kudu/tablet/diskrowset-test.cc| 64 +++
 src/kudu/tablet/diskrowset.cc | 19 -
 src/kudu/tablet/diskrowset.h  | 12 --
 src/kudu/tablet/tablet.cc | 12 +-
 src/kudu/tablet/tablet_history_gc-test.cc | 51 +---
 8 files changed, 167 insertions(+), 12 deletions(-)

diff --git a/src/kudu/tablet/delta_tracker.cc b/src/kudu/tablet/delta_tracker.cc
index 166eebe4c..d336867fd 100644
--- a/src/kudu/tablet/delta_tracker.cc
+++ b/src/kudu/tablet/delta_tracker.cc
@@ -1010,6 +1010,20 @@ int64_t DeltaTracker::CountDeletedRows() const {
   return deleted_row_count_ + (dms_exists_ ? dms_->deleted_row_count() : 0);
 }
 
+int64_t DeltaTracker::CountDeletedRowsInRedos() const {
+  shared_lock lock(component_lock_);
+
+  int64_t delete_count = 0;
+  for (const shared_ptr& ds : redo_delta_stores_) {
+// We won't force open files just to read their stats.
+if (!ds->has_delta_stats()) {
+  continue;
+}
+delete_count += ds->delta_stats().delete_count();
+  }
+  return delete_count;
+}
+
 string DeltaTracker::LogPrefix() const {
   return Substitute("T $0 P $1: ",
 rowset_metadata_->tablet_metadata()->tablet_id(),
diff --git a/src/kudu/tablet/delta_tracker.h b/src/kudu/tablet/delta_tracker.h
index 17717d1f4..d0b454aa7 100644
--- a/src/kudu/tablet/delta_tracker.h
+++ b/src/kudu/tablet/delta_tracker.h
@@ -287,6 +287,11 @@ class DeltaTracker {
   // in a flushing DMS (if one exists)
   int64_t CountDeletedRows() const;
 
+  // Count the number of deleted rows in REDO delta stores per their
+  // delta stats.
+  // Note : we won't force open files just to read their stats.
+  int64_t CountDeletedRowsInRedos() const;
+
  private:
   FRIEND_TEST(TestRowSet, TestRowSetUpdate);
   FRIEND_TEST(TestRowSet, TestDMSFlush);
diff --git a/src/kudu/tablet/diskrowset-test-base.h 
b/src/kudu/tablet/diskrowset-test-base.h
index 23e424bbb..c5ce471df 100644
--- a/src/kudu/tablet/diskrowset-test-base.h
+++ b/src/kudu/tablet/diskrowset-test-base.h
@@ -98,7 +98,7 @@ class TestRowSet : public KuduRowSetTest {
   // The string values are padded out to 15 digits
   void WriteTestRowSet(int n_rows = 0, bool zero_vals = false) {
 DiskRowSetWriter drsw(rowset_meta_.get(), &schema_,
-  BloomFilterSizing::BySizeAndFPRate(32*1024, 0.01f));
+  BloomFilterSizing::BySizeAndFPRate(32*1024, 0.01F));
 DoWriteTestRowSet(n_rows, &drsw, zero_vals);
   }
 
diff --git a/src/kudu/tablet/diskrowset-test.cc 
b/src/kudu/tablet/diskrowset-test.cc
index d645bdaa3..6081dc281 100644
--- a/src/kudu/tablet/diskrowset-test.cc
+++ b/src/kudu/tablet/diskrowset-test.cc
@@ -7