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 065acfbff [log] fix the Ranger client log count limit not working
065acfbff is described below
commit 065acfbff08ab7d77308ebe9f874d9e1d08d608d
Author: kedeng <kdeng...@gmail.com>
AuthorDate: Thu Jul 11 17:37:06 2024 +0800
[log] fix the Ranger client log count limit not working
In scenarios where authentication is enabled, the master
starts a Ranger subprocess to communicate with the KMS
process. I noticed that in these scenarios, the number
of log files generated by the Java subprocess was not
limited, causing a waste of disk space.
To address this issue, I updated the configuration file
for the Java subprocess. To verify the effectiveness of
this commit, I also added new unit tests to ensure it.
Change-Id: Idc528d68947c222fa7be338057ea7af134eb0dd4
Reviewed-on: http://gerrit.cloudera.org:8080/21572
Reviewed-by: Zoltan Chovan <zcho...@cloudera.com>
Reviewed-by: Yingchun Lai <laiyingc...@apache.org>
Tested-by: Yingchun Lai <laiyingc...@apache.org>
Reviewed-by: Alexey Serbin <ale...@apache.org>
---
.../kudu/subprocess/log/LoggingTestMain.java | 33 ++++++++
src/kudu/subprocess/subprocess_proxy-test.cc | 92 ++++++++++++++++++++++
src/kudu/subprocess/subprocess_proxy.cc | 10 ++-
src/kudu/subprocess/subprocess_proxy.h | 1 -
4 files changed, 133 insertions(+), 3 deletions(-)
diff --git
a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/log/LoggingTestMain.java
b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/log/LoggingTestMain.java
new file mode 100644
index 000000000..b2146da72
--- /dev/null
+++
b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/log/LoggingTestMain.java
@@ -0,0 +1,33 @@
+// 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.
+
+package org.apache.kudu.subprocess.log;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+class LoggingTestMain {
+ private static final Logger logger =
LoggerFactory.getLogger(LoggingTestMain.class);
+
+ public static void main(String[] args) throws Exception {
+ for (int i = 0; i < 100000; i++) {
+ logger.debug("This is a test log message number: " + i);
+ }
+ }
+}
diff --git a/src/kudu/subprocess/subprocess_proxy-test.cc
b/src/kudu/subprocess/subprocess_proxy-test.cc
index 00c615b1e..c6b66f0b9 100644
--- a/src/kudu/subprocess/subprocess_proxy-test.cc
+++ b/src/kudu/subprocess/subprocess_proxy-test.cc
@@ -17,15 +17,18 @@
#include "kudu/subprocess/subprocess_proxy.h"
+#include <csignal>
#include <cstddef>
#include <cstdint>
#include <functional>
#include <memory>
+#include <ostream>
#include <string>
#include <utility>
#include <vector>
#include <gflags/gflags_declare.h>
+#include <glog/logging.h>
#include <gtest/gtest.h>
#include "kudu/gutil/casts.h"
@@ -36,8 +39,11 @@
#include "kudu/subprocess/subprocess.pb.h"
#include "kudu/util/env.h"
#include "kudu/util/metrics.h"
+#include "kudu/util/monotime.h"
#include "kudu/util/path_util.h"
+#include "kudu/util/slice.h"
#include "kudu/util/status.h"
+#include "kudu/util/subprocess.h"
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
@@ -64,6 +70,92 @@ using strings::Substitute;
namespace kudu {
namespace subprocess {
+// Helper function to count files in a directory
+int CountLogFiles(const string& log_dir) {
+ vector<string> logfiles;
+ string pattern = Substitute("$0/*.log.gz", log_dir);
+ CHECK_OK(Env::Default()->Glob(pattern, &logfiles));
+ LOG(INFO) << "Found " << logfiles.size() << " log files";
+ return logfiles.size();
+}
+
+class SubprocessProxyTest : public KuduTest {
+ public:
+ SubprocessProxyTest()
+ : test_dir_(GetTestDataDirectory()) {}
+
+ void TearDown() override {
+ if (process_) {
+ process_->KillAndWait(SIGTERM);
+ }
+ KuduTest::TearDown();
+ }
+
+ string GetLogDir() const {
+ return JoinPathSegments(test_dir_, "logs");
+ }
+
+ Status CreateLog4j2PropertiesFile(string* log_properties_path,
+ int log_file_limit) {
+ const string log_dir = GetLogDir();
+ const string log_filename = "kudu-subprocess-log-test-log4j2.properties";
+ const string log4j2_properties_path = JoinPathSegments(log_dir,
log_filename);
+ unique_ptr<WritableFile> writer;
+ RETURN_NOT_OK(env_->CreateDir(log_dir));
+ RETURN_NOT_OK(env_->NewWritableFile(log4j2_properties_path, &writer));
+ string exe;
+ RETURN_NOT_OK(env_->GetExecutablePath(&exe));
+ const string program_name = BaseName(exe);
+ RETURN_NOT_OK(writer->Append(subprocess::Log4j2Properties(
+ program_name, log_dir, log_filename,
+ /* rollover_size_mb */ 1,
+ /* max_files */ log_file_limit,
+ /* log_level */ "debug",
+ /* log_to_stdout */ true)));
+ RETURN_NOT_OK(writer->Sync());
+ RETURN_NOT_OK(writer->Close());
+ *log_properties_path = log4j2_properties_path;
+ return Status::OK();
+ }
+
+ Status ResetEchoSubprocess(int log_file_limit) {
+ string log_properties_path;
+ RETURN_NOT_OK(CreateLog4j2PropertiesFile(&log_properties_path,
log_file_limit));
+
+ string exe;
+ RETURN_NOT_OK(env_->GetExecutablePath(&exe));
+ const string bin_dir = DirName(exe);
+ string java_home;
+ RETURN_NOT_OK(FindHomeDir("java", bin_dir, &java_home));
+ const string pipe_file =
SubprocessServer::FifoPath(JoinPathSegments(test_dir_, "echo_pipe"));
+ vector<string> argv = {
+ Substitute("$0/bin/java", java_home),
+ Substitute("-Dlog4j2.configurationFile=$0", log_properties_path),
+ "-cp", Substitute("$0/kudu-subprocess.jar", bin_dir),
+ "org.apache.kudu.subprocess.log.LoggingTestMain"
+ };
+ process_.reset(new Subprocess(argv));
+ return process_->Start();
+ }
+
+ protected:
+ unique_ptr<Subprocess> process_;
+ const string test_dir_;
+};
+
+TEST_F(SubprocessProxyTest, TestLog4j2LogFileCountLimit) {
+ SKIP_IF_SLOW_NOT_ALLOWED();
+ constexpr int log_file_limit = 2;
+ ASSERT_OK(ResetEchoSubprocess(log_file_limit));
+
+ // Wait for the subprocess to generate log files
+ SleepFor(MonoDelta::FromSeconds(10));
+
+ // Verify the number of log files
+ int log_file_count = CountLogFiles(GetLogDir());
+ EXPECT_LE(log_file_count, log_file_limit);
+}
+
class EchoSubprocessTest : public KuduTest {
public:
EchoSubprocessTest()
diff --git a/src/kudu/subprocess/subprocess_proxy.cc
b/src/kudu/subprocess/subprocess_proxy.cc
index ee8d3a92e..95b5d89ef 100644
--- a/src/kudu/subprocess/subprocess_proxy.cc
+++ b/src/kudu/subprocess/subprocess_proxy.cc
@@ -47,13 +47,19 @@ appender.$0.type = RollingFile
appender.$0.name = $1
appender.$0.layout.type = PatternLayout
appender.$0.layout.pattern = %d{yyyy-MM-dd HH:mm:ss.SSS} [%p - %t] (%F:%L) %m%n
-appender.$0.filename = $2/$3.log
+appender.$0.fileName = $2/$3.log
appender.$0.filePattern = $2/$3.%d{yyyyMMdd-HHmmss}.log.gz
appender.$0.policies.type = Policies
appender.$0.policies.size.type = SizeBasedTriggeringPolicy
-appender.$0.policies.size.size = $4 MB
+appender.$0.policies.size.size = $4MB
appender.$0.strategy.type = DefaultRolloverStrategy
appender.$0.strategy.max = $5
+appender.$0.strategy.action.type = Delete
+appender.$0.strategy.action.basePath = $2
+appender.$0.strategy.action.condition.type = IfFileName
+appender.$0.strategy.action.condition.glob = $3*.log.gz
+appender.$0.strategy.action.condition.nestedCondition.type =
IfAccumulatedFileCount
+appender.$0.strategy.action.condition.nestedCondition.exceeds = $5
)";
// $0: appender instance
diff --git a/src/kudu/subprocess/subprocess_proxy.h
b/src/kudu/subprocess/subprocess_proxy.h
index 586c246b4..c480006cc 100644
--- a/src/kudu/subprocess/subprocess_proxy.h
+++ b/src/kudu/subprocess/subprocess_proxy.h
@@ -23,7 +23,6 @@
#include <vector>
#include <glog/logging.h>
-#include <google/protobuf/any.pb.h>
#include "kudu/common/wire_protocol.h"
#include "kudu/gutil/ref_counted.h"
