You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "SammyVimes (via GitHub)" <gi...@apache.org> on 2023/06/27 11:32:11 UTC
[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2249: IGNITE-19828 Optimize RAFT log encoder/decoder data format
SammyVimes commented on code in PR #2249:
URL: https://github.com/apache/ignite-3/pull/2249#discussion_r1243549222
##########
modules/network/src/main/java/org/apache/ignite/internal/network/direct/stream/DirectByteBufferStreamImplV1.java:
##########
@@ -188,7 +188,7 @@ public void setBuffer(ByteBuffer buf) {
this.buf = buf;
heapArr = buf.isDirect() ? null : buf.array();
- baseOff = buf.isDirect() ? GridUnsafe.bufferAddress(buf) : BYTE_ARR_OFF;
+ baseOff = buf.isDirect() ? GridUnsafe.bufferAddress(buf) : BYTE_ARR_OFF + buf.arrayOffset();
Review Comment:
Probably position should be respected too then (for both types)
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Decoder.java:
##########
@@ -51,98 +52,127 @@ public LogEntry decode(final byte[] content) {
return log;
}
+ // Refactored to look closer to Ignites code style.
public void decode(final LogEntry log, final byte[] content) {
- // 1-5 type
- final int iType = Bits.getInt(content, 1);
- log.setType(EnumOutter.EntryType.forNumber(iType));
- // 5-13 index
- // 13-21 term
- final long index = Bits.getLong(content, 5);
- final long term = Bits.getLong(content, 13);
+ var reader = new Reader(content);
Review Comment:
Probably some of the comments should be retained. Or maybe we can describe the format of the entry here (like we do in storages)
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Encoder.java:
##########
@@ -46,130 +48,137 @@ public byte[] encode(final LogEntry log) {
List<PeerId> oldLearners = log.getOldLearners();
ByteBuffer data = log.getData();
- // magic number 1 byte
- int totalLen = 1;
- final int iType = type.getNumber();
- final long index = id.getIndex();
- final long term = id.getTerm();
- // type(4) + index(8) + term(8) + checksum(8)
- totalLen += 4 + 8 + 8 + 8;
- int peerCount = 0;
- // peer count
- totalLen += 4;
- final List<String> peerStrs = new ArrayList<>();
- if (peers != null) {
- peerCount = peers.size();
- for (final PeerId peer : peers) {
- final String peerStr = peer.toString();
- // peer len (short in 2 bytes)
- // peer str
- totalLen += 2 + peerStr.length();
- peerStrs.add(peerStr);
- }
- }
- int oldPeerCount = 0;
- // old peer count
- totalLen += 4;
- final List<String> oldPeerStrs = new ArrayList<>();
- if (oldPeers != null) {
- oldPeerCount = oldPeers.size();
- for (final PeerId peer : oldPeers) {
- final String peerStr = peer.toString();
- // peer len (short in 2 bytes)
- // peer str
- totalLen += 2 + peerStr.length();
- oldPeerStrs.add(peerStr);
- }
- }
- int learnerCount = 0;
- // peer count
- totalLen += 4;
- final List<String> learnerStrs = new ArrayList<>();
- if (learners != null) {
- learnerCount = learners.size();
- for (final PeerId learner : learners) {
- final String learnerStr = learner.toString();
- // learner len (short in 2 bytes)
- // learner str
- totalLen += 2 + learnerStr.length();
- learnerStrs.add(learnerStr);
- }
- }
- int oldLearnerCount = 0;
- // old peer count
- totalLen += 4;
- final List<String> oldLearnerStrs = new ArrayList<>();
- if (oldLearners != null) {
- oldLearnerCount = oldLearners.size();
- for (final PeerId oldLearner : oldLearners) {
- final String learnerStr = oldLearner.toString();
- // oldLearner len (short in 2 bytes)
- // oldLearner str
- totalLen += 2 + learnerStr.length();
- oldLearnerStrs.add(learnerStr);
- }
+ int totalLen = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
Review Comment:
Same about comments/entry format
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Decoder.java:
##########
@@ -51,98 +52,127 @@ public LogEntry decode(final byte[] content) {
return log;
}
+ // Refactored to look closer to Ignites code style.
public void decode(final LogEntry log, final byte[] content) {
- // 1-5 type
- final int iType = Bits.getInt(content, 1);
- log.setType(EnumOutter.EntryType.forNumber(iType));
- // 5-13 index
- // 13-21 term
- final long index = Bits.getLong(content, 5);
- final long term = Bits.getLong(content, 13);
+ var reader = new Reader(content);
+ reader.pos = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
+
+ int typeNumber = (int)reader.readLong();
+ EnumOutter.EntryType type = Objects.requireNonNull(EnumOutter.EntryType.forNumber(typeNumber));
+ log.setType(type);
+
+ long index = reader.readLong();
+ long term = reader.readLong();
log.setId(new LogId(index, term));
- // 21-29 checksum
- log.setChecksum(Bits.getLong(content, 21));
- // 29-33 peer count
- int peerCount = Bits.getInt(content, 29);
- // peers
- int pos = 33;
- if (peerCount > 0) {
- List<PeerId> peers = new ArrayList<>(peerCount);
- while (peerCount-- > 0) {
- final short len = Bits.getShort(content, pos);
- final byte[] bs = new byte[len];
- System.arraycopy(content, pos + 2, bs, 0, len);
- // peer len (short in 2 bytes)
- // peer str
- pos += 2 + len;
- final PeerId peer = new PeerId();
- peer.parse(AsciiStringUtil.unsafeDecode(bs));
- peers.add(peer);
+
+ long checksum = Bits.getLong(content, reader.pos);
+ log.setChecksum(checksum);
+
+ int pos = reader.pos + Long.BYTES;
+
+ // Peers and learners.
+ if (type != EnumOutter.EntryType.ENTRY_TYPE_DATA) {
+ reader.pos = pos;
+ int peerCount = (int)reader.readLong();
+ pos = reader.pos;
+ if (peerCount > 0) {
+ List<PeerId> peers = new ArrayList<>(peerCount);
+
+ pos = readNodesList(pos, content, peerCount, peers);
+
+ log.setPeers(peers);
}
- log.setPeers(peers);
- }
- // old peers
- int oldPeerCount = Bits.getInt(content, pos);
- pos += 4;
- if (oldPeerCount > 0) {
- List<PeerId> oldPeers = new ArrayList<>(oldPeerCount);
- while (oldPeerCount-- > 0) {
- final short len = Bits.getShort(content, pos);
- final byte[] bs = new byte[len];
- System.arraycopy(content, pos + 2, bs, 0, len);
- // peer len (short in 2 bytes)
- // peer str
- pos += 2 + len;
- final PeerId peer = new PeerId();
- peer.parse(AsciiStringUtil.unsafeDecode(bs));
- oldPeers.add(peer);
+
+ reader.pos = pos;
+ int oldPeerCount = (int)reader.readLong();
+ pos = reader.pos;
+ if (oldPeerCount > 0) {
+ List<PeerId> oldPeers = new ArrayList<>(oldPeerCount);
+
+ pos = readNodesList(pos, content, oldPeerCount, oldPeers);
+
+ log.setOldPeers(oldPeers);
}
- log.setOldPeers(oldPeers);
- }
- // learners
- int learnersCount = Bits.getInt(content, pos);
- pos += 4;
- if (learnersCount > 0) {
- List<PeerId> learners = new ArrayList<>(learnersCount);
- while (learnersCount-- > 0) {
- final short len = Bits.getShort(content, pos);
- final byte[] bs = new byte[len];
- System.arraycopy(content, pos + 2, bs, 0, len);
- // peer len (short in 2 bytes)
- // peer str
- pos += 2 + len;
- final PeerId peer = new PeerId();
- peer.parse(AsciiStringUtil.unsafeDecode(bs));
- learners.add(peer);
+
+ reader.pos = pos;
+ int learnersCount = (int)reader.readLong();
+ pos = reader.pos;
+ if (learnersCount > 0) {
+ List<PeerId> learners = new ArrayList<>(learnersCount);
+
+ pos = readNodesList(pos, content, learnersCount, learners);
+
+ log.setLearners(learners);
+ }
+
+ reader.pos = pos;
+ int oldLearnersCount = (int)reader.readLong();
+ pos = reader.pos;
+ if (oldLearnersCount > 0) {
+ List<PeerId> oldLearners = new ArrayList<>(oldLearnersCount);
+
+ pos = readNodesList(pos, content, oldLearnersCount, oldLearners);
+
+ log.setOldLearners(oldLearners);
}
- log.setLearners(learners);
}
- // old learners
- int oldLearnersCount = Bits.getInt(content, pos);
- pos += 4;
- if (oldLearnersCount > 0) {
- List<PeerId> oldLearners = new ArrayList<>(oldLearnersCount);
- while (oldLearnersCount-- > 0) {
- final short len = Bits.getShort(content, pos);
- final byte[] bs = new byte[len];
- System.arraycopy(content, pos + 2, bs, 0, len);
- // peer len (short in 2 bytes)
- // peer str
- pos += 2 + len;
- final PeerId peer = new PeerId();
- peer.parse(AsciiStringUtil.unsafeDecode(bs));
- oldLearners.add(peer);
+
+ // Data.
+ if (type != EnumOutter.EntryType.ENTRY_TYPE_CONFIGURATION) {
+ if (content.length > pos) {
+ int len = content.length - pos;
+
+ ByteBuffer data = ByteBuffer.wrap(content, pos, len).slice();
+
+ log.setData(data);
}
- log.setOldLearners(oldLearners);
}
+ }
+
+ private static int readNodesList(int pos, byte[] content, int count, List<PeerId> nodes) {
+ while (count --> 0) {
Review Comment:
```suggestion
while (count-- > 0) {
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org