ilikepi63 commented on code in PR #1624:
URL: https://github.com/apache/iggy/pull/1624#discussion_r1986858249
##########
sdk/src/segments/mod.rs:
##########
@@ -0,0 +1,3 @@
+pub mod delete_segments;
+
+const MAX_SEGMENTS_COUNT: u32 = 1000;
Review Comment:
Again, not certain if we want to add a maximum here, but just copied as much
as possible from partitions.
##########
sdk/src/command.rs:
##########
@@ -85,6 +85,8 @@ pub const CREATE_PARTITIONS: &str = "partition.create";
pub const CREATE_PARTITIONS_CODE: u32 = 402;
pub const DELETE_PARTITIONS: &str = "partition.delete";
pub const DELETE_PARTITIONS_CODE: u32 = 403;
+pub const DELETE_SEGMENTS: &str = "segment.delete";
+pub const DELETE_SEGMENTS_CODE: u32 = 404;
Review Comment:
I am not certain if we want this code to correspond to this message.
##########
sdk/src/error.rs:
##########
@@ -232,6 +232,8 @@ pub enum IggyError {
CannotReadTopics(u32) = 2017,
#[error("Invalid replication factor")]
InvalidReplicationFactor = 2018,
+ #[error("Too many segments")]
Review Comment:
I am not certain what the values of segments differ from partitions. I know
that there is a generally configurable segment size on Kafka but not certain if
Iggy follows the same concept?
##########
sdk/src/segments/delete_segments.rs:
##########
@@ -0,0 +1,182 @@
+use crate::bytes_serializable::BytesSerializable;
+use crate::command::{Command, DELETE_SEGMENTS_CODE};
+use crate::error::IggyError;
+use crate::identifier::Identifier;
+use crate::utils::sizeable::Sizeable;
+use crate::validatable::Validatable;
+use bytes::{BufMut, Bytes, BytesMut};
+use serde::{Deserialize, Serialize};
+use std::fmt::Display;
+
+use super::MAX_SEGMENTS_COUNT;
+
+type SegmentsCountType = u32;
Review Comment:
In the delete_partitions command, there is a 4 which I surmise is the byte
length of u32.
I'll try to avoid magic numbers like this so rather chose to do
size_of::<u32>, but if we ever want to change this for whatever reason, we'd
kind of need to change all instances of u32 in this file which might be a
little meh.
That's why I did the type alias :) let me know if you don't think this is
really valuable though
##########
sdk/src/segments/delete_segments.rs:
##########
@@ -0,0 +1,182 @@
+use crate::bytes_serializable::BytesSerializable;
+use crate::command::{Command, DELETE_SEGMENTS_CODE};
+use crate::error::IggyError;
+use crate::identifier::Identifier;
+use crate::utils::sizeable::Sizeable;
+use crate::validatable::Validatable;
+use bytes::{BufMut, Bytes, BytesMut};
+use serde::{Deserialize, Serialize};
+use std::fmt::Display;
+
+use super::MAX_SEGMENTS_COUNT;
+
+type SegmentsCountType = u32;
+
+/// `DeleteSegments` command is used to delete segments from a partition.
+/// It has additional payload:
+/// - `stream_id` - unique stream ID (numeric or name).
+/// - `topic_id` - unique topic ID (numeric or name).
+/// - `partition_id` - unique partition ID (numeric or name).
+/// - `segments_count` - number of segments in the partition to delete, max
value is defined in MAX_SEGMENTS_COUNT.
+#[derive(Debug, Serialize, Deserialize, PartialEq)]
+pub struct DeleteSegments {
+ /// Unique stream ID (numeric or name).
+ #[serde(skip)]
+ pub stream_id: Identifier,
+ /// Unique topic ID (numeric or name).
+ #[serde(skip)]
+ pub topic_id: Identifier,
+ /// Unique partition ID (numeric or name).
+ #[serde(skip)]
+ pub partition_id: Identifier,
+ /// Number of segments in the topic to delete, max value is 1000.
Review Comment:
:white_check_mark:
--
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]
