github-actions[bot] commented on code in PR #4318:
URL: https://github.com/apache/iggy/pull/4318#discussion_r4133165876
##########
foreign/node/src/wire/system/get-stats.command.ts:
##########
@@ -91,6 +99,24 @@ const deserializeGetStats = (b: Buffer) => {
position + 4,
position + 4 + kernelVersionLength
).toString();
+ position += 4 + kernelVersionLength;
+
+ // iggy_server_version, iggy_server_semver
+ const iggyServerVersionLength = b.readUInt32LE(position);
+ position += 4 + iggyServerVersionLength + 4;
+
+ const cacheMetricsCount = b.readUInt32LE(position);
+ position += 4 + cacheMetricsCount * CACHE_METRIC_SIZE +
THREADS_AND_DISK_SIZE;
+ if (position > b.length)
+ deserializeError('stats', position, b.length);
+
+ // Servers that predate the open-files fields end the reply here.
+ let openFilesCount = 0n;
+ let openFilesLimit = 0n;
+ if (b.length > position) {
Review Comment:
nit: `deserializeGetStats` reads the 16 open-files tail bytes whenever any
byte remains, so a reply truncated mid-tail throws a raw `RangeError`. Guard
the tail length and call `deserializeError`, as line 110 does.
##########
core/server_common/src/segment_storage/messages_reader.rs:
##########
@@ -37,6 +39,7 @@ impl MessagesReader {
.read(true)
.open(file_path)
.await
+ .exit_on_descriptor_exhaustion(|| format!("opening {file_path}"))
Review Comment:
warning: `MessagesReader::new` and `IndexReader::new` at
`core/server_common/src/segment_storage/index_reader.rs:41` open read-only to
prove a file exists, yet the added guards exit the process on `EMFILE`.
`core/server_common/src/fatal.rs:83` reserves that exit for opens that write,
create, truncate or sync. Remove both guards, or comment why these probes must
stop the node.
##########
core/metadata/src/impls/metadata.rs:
##########
@@ -2220,6 +2235,33 @@ where
}
}
+ /// Node-wide `[metadata] partitions_max` admission for a `CreateTopic` or
+ /// `CreatePartitions`. Other operations pass.
+ ///
+ /// A soft cap: the apply must not branch on node config, so this counts
+ /// the partitions this primary has committed, and creates in flight
+ /// together can overshoot it. A body that does not decode passes here,
+ /// and `prepare_request` evicts the session for it.
+ fn admit_partitions(&self, message: &Message<RoutedRequestHeader>) ->
Result<(), IggyError> {
Review Comment:
simplification: `admit_partitions` decodes the create body before
`validate_partitions_limit` checks `partitions_max == 0`, so the default
configuration parses every create for a count it discards. Return `Ok(())` when
the cap is zero, before the decode.
--
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]
