Return 0 instead of -1 from hash_table getBytes()
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m25s
CI / pre-commit (pull_request) Successful in 3m40s
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Successful in 5m8s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 4m50s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 4m57s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 4m42s
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Successful in 6m31s
CI / coverage (pull_request) Successful in 4m51s
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m25s
CI / pre-commit (pull_request) Successful in 3m40s
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Successful in 5m8s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 4m50s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 4m57s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 4m42s
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Successful in 6m31s
CI / coverage (pull_request) Successful in 4m51s
The hash_table implementation cannot accurately track memory usage and was returning -1 from ConflictSet::getBytes() and ConflictSet_getBytes(). That violates the API contract that getBytes() returns a non-negative byte count. Change both entry points to return 0 and document in ConflictSet.h that implementations which do not track memory usage may return 0. Add a regression test in test_conflict_set.py that loads the hash_table implementation and verifies getBytes() is non-negative. Closes #62
This commit is contained in:
+8
-3
@@ -96,7 +96,9 @@ void ConflictSet::setOldestVersion(int64_t oldestVersion) {
|
|||||||
return impl->setOldestVersion(oldestVersion);
|
return impl->setOldestVersion(oldestVersion);
|
||||||
}
|
}
|
||||||
|
|
||||||
int64_t ConflictSet::getBytes() const { return -1; }
|
// The hash_table implementation does not track memory usage, so return 0 to
|
||||||
|
// satisfy the API contract that getBytes() returns a non-negative value.
|
||||||
|
int64_t ConflictSet::getBytes() const { return 0; }
|
||||||
|
|
||||||
void ConflictSet::getMetricsV1(MetricsV1 **metrics, int *count) const {
|
void ConflictSet::getMetricsV1(MetricsV1 **metrics, int *count) const {
|
||||||
*metrics = nullptr;
|
*metrics = nullptr;
|
||||||
@@ -161,7 +163,10 @@ __attribute__((__visibility__("default"))) void ConflictSet_destroy(void *cs) {
|
|||||||
}
|
}
|
||||||
__attribute__((__visibility__("default"))) int64_t
|
__attribute__((__visibility__("default"))) int64_t
|
||||||
ConflictSet_getBytes(void *cs) {
|
ConflictSet_getBytes(void *cs) {
|
||||||
using Impl = ConflictSet::Impl;
|
(void)cs;
|
||||||
return -1;
|
// The hash_table implementation does not track memory usage, so return 0 to
|
||||||
|
// satisfy the API contract that ConflictSet_getBytes returns a non-negative
|
||||||
|
// value.
|
||||||
|
return 0;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -88,7 +88,8 @@ struct __attribute__((__visibility__("default"))) ConflictSet {
|
|||||||
|
|
||||||
~ConflictSet();
|
~ConflictSet();
|
||||||
|
|
||||||
/** Returns the total bytes in use by this ConflictSet */
|
/** Returns the total bytes in use by this ConflictSet. Implementations that
|
||||||
|
* do not track memory usage return 0. */
|
||||||
int64_t getBytes() const;
|
int64_t getBytes() const;
|
||||||
|
|
||||||
/** Experimental! */
|
/** Experimental! */
|
||||||
@@ -218,7 +219,8 @@ ConflictSet *ConflictSet_create(int64_t oldestVersion);
|
|||||||
|
|
||||||
void ConflictSet_destroy(ConflictSet *cs);
|
void ConflictSet_destroy(ConflictSet *cs);
|
||||||
|
|
||||||
/** Returns the total bytes in use by this ConflictSet */
|
/** Returns the total bytes in use by this ConflictSet. Implementations that
|
||||||
|
* do not track memory usage return 0. */
|
||||||
int64_t ConflictSet_getBytes(const ConflictSet *cs);
|
int64_t ConflictSet_getBytes(const ConflictSet *cs);
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|||||||
@@ -57,6 +57,17 @@ def test_conflict_set():
|
|||||||
assert cs.check(read(0, key), read(1, key)) == [Result.TOO_OLD, Result.COMMIT]
|
assert cs.check(read(0, key), read(1, key)) == [Result.TOO_OLD, Result.COMMIT]
|
||||||
|
|
||||||
|
|
||||||
|
def test_hash_table_getBytes():
|
||||||
|
# Regression test for issue #62: the hash_table implementation is
|
||||||
|
# point-query only and does not track memory usage, but getBytes() must
|
||||||
|
# still return a non-negative value rather than -1.
|
||||||
|
with ConflictSet(0, build_dir=build_dir, implementation="hash_table") as cs:
|
||||||
|
assert cs.getBytes() == 0
|
||||||
|
cs.addWrites(1, write(b"key"))
|
||||||
|
assert cs.getBytes() >= 0
|
||||||
|
assert cs.check(read(0, b"key")) == [Result.CONFLICT]
|
||||||
|
|
||||||
|
|
||||||
def test_write_read_without_outer_reference():
|
def test_write_read_without_outer_reference():
|
||||||
# Regression test for issue #42: WriteRange/ReadRange must keep their
|
# Regression test for issue #42: WriteRange/ReadRange must keep their
|
||||||
# backing key buffers alive, because the C library reads the pointer
|
# backing key buffers alive, because the C library reads the pointer
|
||||||
|
|||||||
Reference in New Issue
Block a user