MINOR: Prefer MetadataRecordSerde.INSTANCE over new instances#22289
Conversation
github-actions
Bot
added
triage
| try (var snapshot = RecordsSnapshotReader.of( | ||
| raftManager.raftLog().latestSnapshot().get(), | ||
| new MetadataRecordSerde(), | ||
| MetadataRecordSerde.INSTANCE, |
There was a problem hiding this comment.
Would you mind checking the code base?
There was a problem hiding this comment.
I will check all similar code.
JiayaoS
changed the title
|
|
||
| class MetadataRecordSerdeTest { | ||
|
|
||
| private static final MetadataRecordSerde SERDE = MetadataRecordSerde.INSTANCE; |
There was a problem hiding this comment.
Could you remove SERDE and use MetadataRecordSerde.INSTANCE instead?
There was a problem hiding this comment.
Done. Should I change all similar calls to MetadataRecordSerde.INSTANCE?
ApiMessageAndVersion messageAndVersion = SERDE.read(accessor, record.valueSize());
There was a problem hiding this comment.
yes. It is already a static variable :)
There was a problem hiding this comment.
Replaced all direct instantiations of MetadataRecordSerde with
MetadataRecordSerde.INSTANCE, except where the serde is passed in
as a constructor or method parameter (e.g. KafkaRaftManager, BatchAccumulator).
JiayaoS
force-pushed
the
fix/unitTest-RaftClusterSnapshotTest-INSTANCE
branch
from
36321ab to
7d8a956
Compare
JiayaoS
force-pushed
the
fix/unitTest-RaftClusterSnapshotTest-INSTANCE
branch
from
7d8a956 to
2f908f7
Compare
3 participants
Replace
new MetadataRecordSerde()withMetadataRecordSerde.INSTANCEacross the codebase. MetadataRecordSerde is stateless and thread-safe,
so a single shared instance is sufficient.
Also add Javadoc on the public constructor to guide future callers
toward the singleton.
Relevant discussions:
Verified that MetadataRecordSerde (and its parent
AbstractApiMessageSerde) has no mutable instance fields. All methods
(read/write/recordSize) operate solely on their parameters and local
variables, confirming no thread-safety or state-recycling concerns with
sharing a single instance.
Reviewers: Chia-Ping Tsai chia7712@gmail.com