Skip to content

Commit b2cf331

Browse files
ttaylorrgitster
authored andcommitted
bloom: prepare to discard incompatible Bloom filters
Callers use the inline `get_bloom_filter()` implementation as a thin wrapper around `get_or_compute_bloom_filter()`. The former calls the latter with a value of "0" for `compute_if_not_present`, making `get_bloom_filter()` the default read-only path for fetching an existing Bloom filter. Callers expect the value returned from `get_bloom_filter()` is usable, that is that it's compatible with the configured value corresponding to `commitGraph.changedPathsVersion`. This is OK, since the commit-graph machinery only initializes its BDAT chunk (thereby enabling it to service Bloom filter queries) when the Bloom filter hash_version is compatible with our settings. So any value returned by `get_bloom_filter()` is trivially useable. However, subsequent commits will load the BDAT chunk even when the Bloom filters are built with incompatible hash versions. Prepare to handle this by teaching `get_bloom_filter()` to discard filters that are incompatible with the configured hash version. Callers who wish to read incompatible filters (e.g., for upgrading filters from v1 to v2) may use the lower level routine, `get_or_compute_bloom_filter()`. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5b5d5b5 commit b2cf331

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

bloom.c

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,23 @@ static void init_truncated_large_filter(struct bloom_filter *filter,
220220
filter->version = version;
221221
}
222222

223+
struct bloom_filter *get_bloom_filter(struct repository *r, struct commit *c)
224+
{
225+
struct bloom_filter *filter;
226+
int hash_version;
227+
228+
filter = get_or_compute_bloom_filter(r, c, 0, NULL, NULL);
229+
if (!filter)
230+
return NULL;
231+
232+
prepare_repo_settings(r);
233+
hash_version = r->settings.commit_graph_changed_paths_version;
234+
235+
if (!(hash_version == -1 || hash_version == filter->version))
236+
return NULL; /* unusable filter */
237+
return filter;
238+
}
239+
223240
struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
224241
struct commit *c,
225242
int compute_if_not_present,
@@ -245,7 +262,8 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
245262
filter, graph_pos);
246263
}
247264

248-
if (filter->data && filter->len)
265+
if ((filter->data && filter->len) &&
266+
(!settings || settings->hash_version == filter->version))
249267
return filter;
250268
if (!compute_if_not_present)
251269
return NULL;

bloom.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,24 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
108108
const struct bloom_filter_settings *settings,
109109
enum bloom_filter_computed *computed);
110110

111-
#define get_bloom_filter(r, c) get_or_compute_bloom_filter( \
112-
(r), (c), 0, NULL, NULL)
111+
/*
112+
* Find the Bloom filter associated with the given commit "c".
113+
*
114+
* If any of the following are true
115+
*
116+
* - the repository does not have a commit-graph, or
117+
* - the repository disables reading from the commit-graph, or
118+
* - the given commit does not have a Bloom filter computed, or
119+
* - there is a Bloom filter for commit "c", but it cannot be read
120+
* because the filter uses an incompatible version of murmur3
121+
*
122+
* , then `get_bloom_filter()` will return NULL. Otherwise, the corresponding
123+
* Bloom filter will be returned.
124+
*
125+
* For callers who wish to inspect Bloom filters with incompatible hash
126+
* versions, use get_or_compute_bloom_filter().
127+
*/
128+
struct bloom_filter *get_bloom_filter(struct repository *r, struct commit *c);
113129

114130
int bloom_filter_contains(const struct bloom_filter *filter,
115131
const struct bloom_key *key,

0 commit comments

Comments
 (0)