Remove boundaryAndExist (#8994)

* Remove boundaryAndExist

Client knows whether an entry is a boundary, and client also can
see whether the record for that entry exist by checking whether
the rangeResult is empty. This field is useless.

This field breaks backward compatibility, need to remove it.
This commit is contained in:
Hao Fu 2022-12-07 18:08:15 -08:00 committed by GitHub
parent 38972b199a
commit d11b5b44e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 16 additions and 63 deletions

View File

@ -169,7 +169,6 @@ typedef struct mappedkeyvalue {
* take the shortcut. */
FDBGetRangeReqAndResult getRange;
unsigned char buffer[32];
fdb_bool_t boundaryAndExist;
} FDBMappedKeyValue;
#pragma pack(push, 4)

View File

@ -182,17 +182,14 @@ struct GetMappedRangeResult {
const std::string& value,
const std::string& begin,
const std::string& end,
const std::vector<std::pair<std::string, std::string>>& range_results,
fdb_bool_t boundaryAndExist)
: key(key), value(value), begin(begin), end(end), range_results(range_results),
boundaryAndExist(boundaryAndExist) {}
const std::vector<std::pair<std::string, std::string>>& range_results)
: key(key), value(value), begin(begin), end(end), range_results(range_results) {}
std::string key;
std::string value;
std::string begin;
std::string end;
std::vector<std::pair<std::string, std::string>> range_results;
fdb_bool_t boundaryAndExist;
};
std::vector<MappedKV> mkvs;
// True if values remain in the key range requested.
@ -317,7 +314,6 @@ GetMappedRangeResult get_mapped_range(fdb::Transaction& tr,
auto value = extractString(mkv.value);
auto begin = extractString(mkv.getRange.begin.key);
auto end = extractString(mkv.getRange.end.key);
bool boundaryAndExist = mkv.boundaryAndExist;
// std::cout << "key:" << key << " value:" << value << " begin:" << begin << " end:" << end << std::endl;
std::vector<std::pair<std::string, std::string>> range_results;
@ -328,7 +324,7 @@ GetMappedRangeResult get_mapped_range(fdb::Transaction& tr,
range_results.emplace_back(k, v);
// std::cout << "[" << i << "]" << k << " -> " << v << std::endl;
}
result.mkvs.emplace_back(key, value, begin, end, range_results, boundaryAndExist);
result.mkvs.emplace_back(key, value, begin, end, range_results);
}
return result;
}
@ -1096,9 +1092,7 @@ TEST_CASE("fdb_transaction_get_mapped_range") {
CHECK(!result.more);
int id = beginId;
bool boundary;
for (int i = 0; i < expectSize; i++, id++) {
boundary = i == 0 || i == expectSize - 1;
const auto& mkv = result.mkvs[i];
if (matchIndex == MATCH_INDEX_ALL || i == 0 || i == expectSize - 1) {
CHECK(indexEntryKey(id).compare(mkv.key) == 0);
@ -1109,8 +1103,6 @@ TEST_CASE("fdb_transaction_get_mapped_range") {
} else {
CHECK(EMPTY.compare(mkv.key) == 0);
}
bool empty = mkv.range_results.empty();
CHECK(mkv.boundaryAndExist == (boundary && !empty));
CHECK(EMPTY.compare(mkv.value) == 0);
CHECK(mkv.range_results.size() == SPLIT_SIZE);
for (int split = 0; split < SPLIT_SIZE; split++) {
@ -1154,9 +1146,7 @@ TEST_CASE("fdb_transaction_get_mapped_range_missing_all_secondary") {
CHECK(!result.more);
int id = beginId;
bool boundary;
for (int i = 0; i < expectSize; i++, id++) {
boundary = i == 0 || i == expectSize - 1;
const auto& mkv = result.mkvs[i];
if (matchIndex == MATCH_INDEX_ALL || i == 0 || i == expectSize - 1) {
CHECK(indexEntryKey(id).compare(mkv.key) == 0);
@ -1167,8 +1157,6 @@ TEST_CASE("fdb_transaction_get_mapped_range_missing_all_secondary") {
} else {
CHECK(EMPTY.compare(mkv.key) == 0);
}
bool empty = mkv.range_results.empty();
CHECK(mkv.boundaryAndExist == (boundary && !empty));
CHECK(EMPTY.compare(mkv.value) == 0);
}
break;

View File

@ -612,14 +612,14 @@ JNIEXPORT jobject JNICALL Java_com_apple_foundationdb_FutureMappedResults_Future
FDBMappedKeyValue kvm = kvms[i];
int kvm_count = kvm.getRange.m_size;
// now it has 5 field, key, value, getRange.begin, getRange.end, boundaryAndExist
// now it has 4 field, key, value, getRange.begin, getRange.end
// this needs to change if FDBMappedKeyValue definition is changed.
const int totalFieldFDBMappedKeyValue = 5;
const int totalFieldFDBMappedKeyValue = 4;
const int totalLengths = totalFieldFDBMappedKeyValue + kvm_count * 2;
int totalBytes = kvm.key.key_length + kvm.value.key_length + kvm.getRange.begin.key.key_length +
kvm.getRange.end.key.key_length + sizeof(kvm.boundaryAndExist);
kvm.getRange.end.key.key_length;
for (int i = 0; i < kvm_count; i++) {
auto kv = kvm.getRange.data[i];
totalBytes += kv.key_length + kv.value_length;
@ -663,7 +663,6 @@ JNIEXPORT jobject JNICALL Java_com_apple_foundationdb_FutureMappedResults_Future
cpBytesAndLength(pByte, pLength, kvm.value);
cpBytesAndLength(pByte, pLength, kvm.getRange.begin.key);
cpBytesAndLength(pByte, pLength, kvm.getRange.end.key);
cpBytesAndLengthInner(pByte, pLength, (uint8_t*)&(kvm.boundaryAndExist), sizeof(kvm.boundaryAndExist));
for (int kvm_i = 0; kvm_i < kvm_count; kvm_i++) {
auto kv = kvm.getRange.data[kvm_i];
cpBytesAndLengthInner(pByte, pLength, kv.key, kv.key_length);

View File

@ -209,11 +209,6 @@ class MappedRangeQueryIntegrationTest {
assertByteArrayEquals(indexEntryKey(id), mappedKeyValue.getKey());
assertByteArrayEquals(EMPTY, mappedKeyValue.getValue());
assertByteArrayEquals(indexEntryKey(id), mappedKeyValue.getKey());
if (id == begin || id == end - 1) {
Assertions.assertTrue(mappedKeyValue.getBoundaryAndExist());
} else {
Assertions.assertFalse(mappedKeyValue.getBoundaryAndExist());
}
byte[] prefix = recordKeyPrefix(id);
assertByteArrayEquals(prefix, mappedKeyValue.getRangeBegin());
prefix[prefix.length - 1] = (byte)0x01;

View File

@ -33,27 +33,22 @@ public class MappedKeyValue extends KeyValue {
private final byte[] rangeBegin;
private final byte[] rangeEnd;
private final List<KeyValue> rangeResult;
private final int boundaryAndExist;
// now it has 5 field, key, value, getRange.begin, getRange.end, boundaryAndExist
// now it has 4 fields, key, value, getRange.begin, getRange.end
// this needs to change if FDBMappedKeyValue definition is changed.
private static final int TOTAL_SERIALIZED_FIELD_FDBMappedKeyValue = 5;
private static final int TOTAL_SERIALIZED_FIELD_FDBMappedKeyValue = 4;
public MappedKeyValue(byte[] key, byte[] value, byte[] rangeBegin, byte[] rangeEnd, List<KeyValue> rangeResult,
int boundaryAndExist) {
public MappedKeyValue(byte[] key, byte[] value, byte[] rangeBegin, byte[] rangeEnd, List<KeyValue> rangeResult) {
super(key, value);
this.rangeBegin = rangeBegin;
this.rangeEnd = rangeEnd;
this.rangeResult = rangeResult;
this.boundaryAndExist = boundaryAndExist;
}
public byte[] getRangeBegin() { return rangeBegin; }
public byte[] getRangeEnd() { return rangeEnd; }
public boolean getBoundaryAndExist() { return boundaryAndExist == 0 ? false : true; }
public List<KeyValue> getRangeResult() { return rangeResult; }
public static MappedKeyValue fromBytes(byte[] bytes, int[] lengths) {
@ -69,8 +64,6 @@ public class MappedKeyValue extends KeyValue {
byte[] value = takeBytes(offset, bytes, lengths);
byte[] rangeBegin = takeBytes(offset, bytes, lengths);
byte[] rangeEnd = takeBytes(offset, bytes, lengths);
byte[] boundaryAndExistBytes = takeBytes(offset, bytes, lengths);
int boundaryAndExist = ByteBuffer.wrap(boundaryAndExistBytes).order(ByteOrder.LITTLE_ENDIAN).getInt();
if ((lengths.length - TOTAL_SERIALIZED_FIELD_FDBMappedKeyValue) % 2 != 0) {
throw new IllegalArgumentException("There needs to be an even number of lengths!");
@ -82,7 +75,7 @@ public class MappedKeyValue extends KeyValue {
byte[] v = takeBytes(offset, bytes, lengths);
rangeResult.add(new KeyValue(k, v));
}
return new MappedKeyValue(key, value, rangeBegin, rangeEnd, rangeResult, boundaryAndExist);
return new MappedKeyValue(key, value, rangeBegin, rangeEnd, rangeResult);
}
static class Offset {
@ -109,17 +102,14 @@ public class MappedKeyValue extends KeyValue {
return false;
MappedKeyValue rhs = (MappedKeyValue) obj;
return Arrays.equals(rangeBegin, rhs.rangeBegin)
&& Arrays.equals(rangeEnd, rhs.rangeEnd)
&& Objects.equals(rangeResult, rhs.rangeResult)
&& boundaryAndExist == rhs.boundaryAndExist;
return Arrays.equals(rangeBegin, rhs.rangeBegin) && Arrays.equals(rangeEnd, rhs.rangeEnd) &&
Objects.equals(rangeResult, rhs.rangeResult);
}
@Override
public int hashCode() {
int hashForResult = rangeResult == null ? 0 : rangeResult.hashCode();
return 17 +
(29 * hashForResult + boundaryAndExist + 37 * Arrays.hashCode(rangeBegin) + Arrays.hashCode(rangeEnd));
return 17 + (29 * hashForResult + 37 * Arrays.hashCode(rangeBegin) + Arrays.hashCode(rangeEnd));
}
@Override
@ -128,7 +118,6 @@ public class MappedKeyValue extends KeyValue {
sb.append("rangeBegin=").append(ByteArrayUtil.printable(rangeBegin));
sb.append(", rangeEnd=").append(ByteArrayUtil.printable(rangeEnd));
sb.append(", rangeResult=").append(rangeResult);
sb.append(", boundaryAndExist=").append(boundaryAndExist);
sb.append('}');
return super.toString() + "->" + sb.toString();
}

View File

@ -51,8 +51,6 @@ class MappedRangeResultDirectBufferIterator extends DirectBufferIterator impleme
final byte[] value = getString();
final byte[] rangeBegin = getString();
final byte[] rangeEnd = getString();
final byte[] boundaryAndExistBytes = getString();
final int boundaryAndExist = ByteBuffer.wrap(boundaryAndExistBytes).getInt();
final int rangeResultSize = byteBuffer.getInt();
List<KeyValue> rangeResult = new ArrayList();
for (int i = 0; i < rangeResultSize; i++) {
@ -61,7 +59,7 @@ class MappedRangeResultDirectBufferIterator extends DirectBufferIterator impleme
rangeResult.add(new KeyValue(k, v));
}
current += 1;
return new MappedKeyValue(key, value, rangeBegin, rangeEnd, rangeResult, boundaryAndExist);
return new MappedKeyValue(key, value, rangeBegin, rangeEnd, rangeResult);
}
private byte[] getString() {

View File

@ -4283,7 +4283,6 @@ int64_t inline getRangeResultFamilyBytes(MappedRangeResultRef result) {
int64_t bytes = 0;
for (const MappedKeyValueRef& mappedKeyValue : result) {
bytes += mappedKeyValue.key.size() + mappedKeyValue.value.size();
bytes += sizeof(mappedKeyValue.boundaryAndExist);
auto& reqAndResult = mappedKeyValue.reqAndResult;
if (std::holds_alternative<GetValueReqAndResultRef>(reqAndResult)) {
auto getValue = std::get<GetValueReqAndResultRef>(reqAndResult);

View File

@ -814,18 +814,9 @@ struct MappedKeyValueRef : KeyValueRef {
MappedReqAndResultRef reqAndResult;
// boundary KVs are always returned so that caller can use it as a continuation,
// for non-boundary KV, it is always false.
// for boundary KV, it is true only when the secondary query succeeds(return non-empty).
// Note: only MATCH_INDEX_MATCHED_ONLY and MATCH_INDEX_UNMATCHED_ONLY modes can make use of it,
// to decide whether the boudnary is a match/unmatch.
// In the case of MATCH_INDEX_ALL and MATCH_INDEX_NONE, caller should not care if boundary has a match or not.
bool boundaryAndExist;
MappedKeyValueRef() = default;
MappedKeyValueRef(Arena& a, const MappedKeyValueRef& copyFrom) : KeyValueRef(a, copyFrom) {
const auto& reqAndResultCopyFrom = copyFrom.reqAndResult;
boundaryAndExist = copyFrom.boundaryAndExist;
if (std::holds_alternative<GetValueReqAndResultRef>(reqAndResultCopyFrom)) {
auto getValue = std::get<GetValueReqAndResultRef>(reqAndResultCopyFrom);
reqAndResult = GetValueReqAndResultRef(a, getValue);
@ -839,7 +830,7 @@ struct MappedKeyValueRef : KeyValueRef {
bool operator==(const MappedKeyValueRef& rhs) const {
return static_cast<const KeyValueRef&>(*this) == static_cast<const KeyValueRef&>(rhs) &&
reqAndResult == rhs.reqAndResult && boundaryAndExist == rhs.boundaryAndExist;
reqAndResult == rhs.reqAndResult;
}
bool operator!=(const MappedKeyValueRef& rhs) const { return !(rhs == *this); }
@ -849,7 +840,7 @@ struct MappedKeyValueRef : KeyValueRef {
template <class Ar>
void serialize(Ar& ar) {
serializer(ar, ((KeyValueRef&)*this), reqAndResult, boundaryAndExist);
serializer(ar, ((KeyValueRef&)*this), reqAndResult);
}
};

View File

@ -80,7 +80,6 @@ struct FdbCApi : public ThreadSafeReferenceCounted<FdbCApi> {
* and take the shortcut. */
FDBGetRangeReqAndResult getRange;
unsigned char buffer[32];
bool boundaryAndExist;
} FDBMappedKeyValue;
#pragma pack(push, 4)

View File

@ -4874,13 +4874,9 @@ ACTOR Future<GetMappedKeyValuesReply> mapKeyValues(StorageServer* data,
// keep index for boundary index entries, so that caller can use it as a continuation.
result.data[0].key = input.data[0].key;
result.data[0].value = input.data[0].value;
result.data[0].boundaryAndExist = getMappedKeyValueSize(kvms[0]) > 0;
result.data.back().key = input.data[resultSize - 1].key;
result.data.back().value = input.data[resultSize - 1].value;
// index needs to be -1
int index = (resultSize - 1) % SERVER_KNOBS->MAX_PARALLEL_QUICK_GET_VALUE;
result.data.back().boundaryAndExist = getMappedKeyValueSize(kvms[index]) > 0;
}
result.more = input.more || resultSize < sz;
if (pOriginalReq->options.present() && pOriginalReq->options.get().debugID.present())