acelyc111 commented on code in PR #1594:
URL:
https://github.com/apache/incubator-pegasus/pull/1594#discussion_r1323168018
##########
src/replica/mutation_log.h:
##########
@@ -345,6 +345,67 @@ class mutation_log : public ref_counter
// get total size ithout lock.
int64_t total_size_no_lock() const;
+ struct reserved_slog_info
+ {
+ size_t file_count;
+ int64_t log_size;
+ int smallest_file_index;
+ int largest_file_index;
Review Comment:
How about renaming "min" and "max"?
##########
src/replica/mutation_log.h:
##########
@@ -345,6 +345,67 @@ class mutation_log : public ref_counter
// get total size ithout lock.
int64_t total_size_no_lock() const;
+ struct reserved_slog_info
+ {
+ size_t file_count;
+ int64_t log_size;
+ int smallest_file_index;
+ int largest_file_index;
Review Comment:
Initialize the variables to 0 ?
##########
src/replica/mutation_log.h:
##########
@@ -345,6 +345,67 @@ class mutation_log : public ref_counter
// get total size ithout lock.
int64_t total_size_no_lock() const;
+ struct reserved_slog_info
+ {
+ size_t file_count;
+ int64_t log_size;
+ int smallest_file_index;
+ int largest_file_index;
+
+ std::string to_string() const
+ {
+ return fmt::format("reserved_slog_info = [file_count = {},
log_size = {}, "
+ "smallest_file_index = {}, largest_file_index =
{}]",
+ file_count,
+ log_size,
+ smallest_file_index,
+ largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
reserved_slog_info &reserved_log)
+ {
+ return os << reserved_log.to_string();
+ }
+ };
Review Comment:
It's needed to add USER_DEFINED_STRUCTURE_FORMATTER for reserved_slog_info
because we have upgraded libfmt.
##########
src/replica/mutation_log.h:
##########
@@ -345,6 +345,67 @@ class mutation_log : public ref_counter
// get total size ithout lock.
int64_t total_size_no_lock() const;
+ struct reserved_slog_info
+ {
+ size_t file_count;
+ int64_t log_size;
+ int smallest_file_index;
+ int largest_file_index;
+
+ std::string to_string() const
+ {
+ return fmt::format("reserved_slog_info = [file_count = {},
log_size = {}, "
+ "smallest_file_index = {}, largest_file_index =
{}]",
+ file_count,
+ log_size,
+ smallest_file_index,
+ largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
reserved_slog_info &reserved_log)
+ {
+ return os << reserved_log.to_string();
+ }
+ };
+
+ struct slog_deletion_info
+ {
+ int to_delete_file_count = 0;
+ int64_t to_delete_log_size = 0;
+ int deleted_file_count = 0;
+ int64_t deleted_log_size = 0;
+ int deleted_smallest_file_index = 0;
+ int deleted_largest_file_index = 0;
Review Comment:
Same, rename to "min" and "max"?
##########
src/replica/mutation_log.h:
##########
@@ -345,6 +345,67 @@ class mutation_log : public ref_counter
// get total size ithout lock.
int64_t total_size_no_lock() const;
+ struct reserved_slog_info
+ {
+ size_t file_count;
+ int64_t log_size;
+ int smallest_file_index;
+ int largest_file_index;
+
+ std::string to_string() const
+ {
+ return fmt::format("reserved_slog_info = [file_count = {},
log_size = {}, "
+ "smallest_file_index = {}, largest_file_index =
{}]",
+ file_count,
+ log_size,
+ smallest_file_index,
+ largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
reserved_slog_info &reserved_log)
+ {
+ return os << reserved_log.to_string();
+ }
+ };
+
+ struct slog_deletion_info
+ {
+ int to_delete_file_count = 0;
+ int64_t to_delete_log_size = 0;
+ int deleted_file_count = 0;
+ int64_t deleted_log_size = 0;
+ int deleted_smallest_file_index = 0;
+ int deleted_largest_file_index = 0;
+
+ std::string to_string() const
+ {
+ return fmt::format("slog_deletion_info = [to_delete_file_count =
{}, "
+ "to_delete_log_size = {}, deleted_file_count =
{}, "
+ "deleted_log_size = {},
deleted_smallest_file_index = {}, "
+ "deleted_largest_file_index = {}]",
+ to_delete_file_count,
+ to_delete_log_size,
+ deleted_file_count,
+ deleted_log_size,
+ deleted_smallest_file_index,
+ deleted_largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
slog_deletion_info &log_deletion)
+ {
+ return os << log_deletion.to_string();
+ }
+ };
Review Comment:
Same.
##########
src/replica/mutation_log.h:
##########
@@ -345,6 +345,67 @@ class mutation_log : public ref_counter
// get total size ithout lock.
int64_t total_size_no_lock() const;
+ struct reserved_slog_info
+ {
+ size_t file_count;
+ int64_t log_size;
+ int smallest_file_index;
+ int largest_file_index;
+
+ std::string to_string() const
+ {
+ return fmt::format("reserved_slog_info = [file_count = {},
log_size = {}, "
+ "smallest_file_index = {}, largest_file_index =
{}]",
+ file_count,
+ log_size,
+ smallest_file_index,
+ largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
reserved_slog_info &reserved_log)
+ {
+ return os << reserved_log.to_string();
+ }
+ };
+
+ struct slog_deletion_info
+ {
+ int to_delete_file_count = 0;
+ int64_t to_delete_log_size = 0;
+ int deleted_file_count = 0;
+ int64_t deleted_log_size = 0;
+ int deleted_smallest_file_index = 0;
+ int deleted_largest_file_index = 0;
+
+ std::string to_string() const
+ {
+ return fmt::format("slog_deletion_info = [to_delete_file_count =
{}, "
+ "to_delete_log_size = {}, deleted_file_count =
{}, "
+ "deleted_log_size = {},
deleted_smallest_file_index = {}, "
+ "deleted_largest_file_index = {}]",
+ to_delete_file_count,
+ to_delete_log_size,
+ deleted_file_count,
+ deleted_log_size,
+ deleted_smallest_file_index,
+ deleted_largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
slog_deletion_info &log_deletion)
+ {
+ return os << log_deletion.to_string();
+ }
+ };
+
+ using log_file_map = std::map<int, log_file_ptr>;
Review Comment:
How about renaming to `log_file_by_index` to indicate the key and value?
##########
src/replica/replica_stub.h:
##########
@@ -435,6 +453,9 @@ class replica_stub : public serverlet<replica_stub>, public
ref_counter
opening_replicas _opening_replicas;
closing_replicas _closing_replicas;
closed_replicas _closed_replicas;
+ size_t _last_prevent_gc_replica_count;
Review Comment:
Could you please add some comments for the 3 newly added member variables?
##########
src/replica/replica_stub.h:
##########
@@ -361,6 +364,20 @@ class replica_stub : public serverlet<replica_stub>,
public ref_counter
replica_life_cycle get_replica_life_cycle(gpid id);
void on_gc_replica(replica_stub_ptr this_, gpid id);
+ struct replica_gc_info
+ {
+ replica_ptr rep;
+ partition_status::type status;
+ mutation_log_ptr plog;
+ decree last_durable_decree;
+ int64_t init_offset_in_shared_log;
+ };
+ using replica_gc_info_map = std::unordered_map<gpid, replica_gc_info>;
+ void gc_slog(const replica_gc_info_map &replica_gc_map);
Review Comment:
Could you please add some comments for the 3 newly added functions?
##########
src/replica/mutation_log.h:
##########
@@ -345,6 +345,67 @@ class mutation_log : public ref_counter
// get total size ithout lock.
int64_t total_size_no_lock() const;
+ struct reserved_slog_info
+ {
+ size_t file_count;
+ int64_t log_size;
+ int smallest_file_index;
+ int largest_file_index;
+
+ std::string to_string() const
+ {
+ return fmt::format("reserved_slog_info = [file_count = {},
log_size = {}, "
+ "smallest_file_index = {}, largest_file_index =
{}]",
+ file_count,
+ log_size,
+ smallest_file_index,
+ largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
reserved_slog_info &reserved_log)
+ {
+ return os << reserved_log.to_string();
+ }
+ };
+
+ struct slog_deletion_info
+ {
+ int to_delete_file_count = 0;
+ int64_t to_delete_log_size = 0;
+ int deleted_file_count = 0;
+ int64_t deleted_log_size = 0;
+ int deleted_smallest_file_index = 0;
+ int deleted_largest_file_index = 0;
+
+ std::string to_string() const
+ {
+ return fmt::format("slog_deletion_info = [to_delete_file_count =
{}, "
+ "to_delete_log_size = {}, deleted_file_count =
{}, "
+ "deleted_log_size = {},
deleted_smallest_file_index = {}, "
+ "deleted_largest_file_index = {}]",
+ to_delete_file_count,
+ to_delete_log_size,
+ deleted_file_count,
+ deleted_log_size,
+ deleted_smallest_file_index,
+ deleted_largest_file_index);
+ }
+
+ friend std::ostream &operator<<(std::ostream &os, const
slog_deletion_info &log_deletion)
+ {
+ return os << log_deletion.to_string();
+ }
+ };
+
+ using log_file_map = std::map<int, log_file_ptr>;
+
+ // Closing and remove all of slog files that are smaller (i.e. older) than
the largest
+ // file index.
Review Comment:
It would be nice to mention if the `largest_file_index_to_delete` is
included or excluded.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
