mqliang commented on pull request #6710:
URL: https://github.com/apache/incubator-pinot/pull/6710#issuecomment-806028103
@mcvsubbu Just found a defect of using enum value as key and encode trailer
as `(int, int, bytes/blob in utf-8) `:
* We are able to add new key into the enum, without break back-compt
* We are able to not include a key into trailer, without break back-compt
* **However, we are unable to remove a key from the enum (if the key is no
long used in a future version)**
Namely, say we now have three keys:
```
// old version:
enum {
key1,
key2,
key3,
}
```
Now if we remove key2 from the enum since it's no longer been used.
```
// new version
enum {
key1,
key3,
}
```
Then, when new broker receive bytes from old server, it will interpret value
of k2 as value of k3.
So a better solution is using string as key and encode trailer as `(int of
key length, bytes of key in utf-8, int of value length, bytes of value in
utf-8)`. Which is exactly how we encode metadata in V2.
However, if we do it in his way, it's equivalent to just moving metadata
section to the end of datatable, which does not make too much sense to bump up
a version just for rearrange sections in datatable.
Let's take a step back to what we wanner solve:
* we wanner add serialization_cost to datatable, but serialization_cost is
not available before serialization.
* we wanner keep back-comp
To add serialization_cost to datatable after serialization, basically we
have two options:
* append it to the end of bytes.
* put a temporary value of serialization_cost when serialization, after
serialization is done, replace it as the actual value.
So, here is another approach:
* don't add a trailer section
* put serialization_cost into metedata
* we serialize metedata, in V2 we encode it as `(int of key length, bytes of
key in utf-8, int of value length, bytes of value in utf-8)`. Encoding in this
way makes value replacement after serialization impossible, since
`String.valueOf("1000").length() != String.valueOf("100000").length()`.
* In V3, keep all existing logic. However, if the value is long, we should
encode it as `(int of key length, bytes of key in utf-8,
toBigEndian(longValue))`. And the the function of `serializaMetadata()`, we can
have a variable to record the start offset of serialization_cost.
```
bytes[] bytes;
int serialization_cost_value_start_offset;
offset = 0;
for (String key: metadata.keySet()) {
keybytes[] = to-utf8(key);
bytes.append(keybytes.length())
bytes.append(keybytes)
offset += 4;
offset += keybytes.length
valuebytes[]
if (key.equals("erialization_cost")) {
serialization_cost_value_start_offset = offset;
valuebytes = toBigEndian(value);
} else {
valuebytes = to-utf8(value);
}
bytes.append(valuebytes.length())
bytes.append(valuebytes)
offset += 4;
offset += keybytes.length
}
```
So after serialization, we are able to replace the value of
serialization_cost (`toBigEndian(longValue)` is always 8 bytes, which makes
replacement possible):
```
offset = metadataStartOffset+serialization_cost_value_start_offset
bytes[offset:offset+8] = toBigEndian(actualValue)
```
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
