git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Teng Long <dyroneteng@gmail.com>
To: dyroneteng@gmail.com
Cc: avarab@gmail.com, derrickstolee@github.com, git@vger.kernel.org,
	gitster@pobox.com, me@ttaylorr.com, tenglong.tl@alibaba-inc.com
Subject: [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file
Date: Tue, 21 Jun 2022 21:25:05 +0800	[thread overview]
Message-ID: <8735ae99798a43650acb9a1df7ba2072c7758476.1655817253.git.dyroneteng@gmail.com> (raw)
In-Reply-To: <cover.1655817253.git.dyroneteng@gmail.com>

It's supported for a repo to use bitmap in both single-pack bitmap
way or a multi-pack(MIDX) bitmap. Either of two bitmap kinds can
exist in the repository, or both can be stored but let the config
controls which kind of bitmap is used (like "core.multipackIndex",
etc.). Because of this, sometimes the bitmap debug path is not
obvious enough, for example, when executing:

 $ git rev-list  --test-bitmap  HEAD
 fatal: failed to load bitmap indexes

When the output look like this, it's unclear which kind(s) of
.bitmap exist, and which were read. For example, it's possible a MIDX
bitmap exists, but was not read (e.g., because
core.multiPackIndex=false), among many other scenarios.

Therefore, we added some trace2 code so that when we read the bitmap
we can be more clear about the decision path, such as whether it is
working on MIDX or single-pack bitmap at present, or the related config
is enabled or not. This may help with logging, user troubleshooting, and
development debugging.

Here are some brief output (omitted some unrelated or repetitive rows
and columns, using "..." instead of) examples on two scenarios when
executing:

  $ GIT_TRACE2_PERF=1 git rev-list --test-bitmap HEAD

Scenario 1:
  core.multipackIndex [false], MIDX bitmap exists [Y], single-pack bitmap exists [N]

  ...
  ... | main                     | data         | r1  | ... | config       | core.multipackindex:false
  ... | d0 | main                | region_enter | r1  | ... | pack-bitmap  | label:open_bitmap
  ... | d0 | main                | data         | r1  | ... | bitmap       | ..path:.git/objects/pack/pack-e9eb18e6a423057f4424a762069e13804a75d01e.bitmap
  ... | main                     | region_leave | r1  | ... | pack-bitmap  | label:open_bitmap
  ... | main                     | error        |     | ... |              | failed to load bitmap indexes
  fatal: failed to load bitmap indexes
  ... | d0 | main                | exit         |     | ... |              | code:128
  ...

Scenario 2:
  core.multipackIndex [false], MIDX bitmap exists [Y], single-pack bitmap exists [Y]

  ... | d0 | main                | region_enter | r0  | ... | pack-bitmap  | label:open_bitmap
  ... | d0 | main                | data         | r0  | ... | bitmap       | ..path:/home/tenglong.tl/test/dyrone_bitmap/.git/objects/pack/pack-e9eb18e6a423057f4424a762069e13804a75d01e.bitmap
  ... | main                     | region_leave | r0  | ... | pack-bitmap  | label:open_bitmap
  Bitmap v1 test (1 entries loaded)
  Found bitmap for d864fefa87415d6cd289c72aa9ffd45b4a8ffd84. 64 bits / 11030517 checksum
  ... | main                     | region_enter | r0  | ... | progress     | label:Verifying bitmap entries
  Verifying bitmap entries: 100% (3/3), done.
  ... | main                     | data         | r0  | ... | progress     | ..total_objects:3
  ... | main                     | region_leave | r0  | ... | progress     | label:Verifying bitmap entries
  OK!
  ... | d0 | main                | exit         |     | ... |              | code:0
  ...

Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
 pack-bitmap.c   | 21 ++++++++++++++++-----
 repo-settings.c | 22 ++++++++++++----------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/pack-bitmap.c b/pack-bitmap.c
index a54d5a0c9f..c39d722592 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -312,9 +312,12 @@ char *pack_bitmap_filename(struct packed_git *p)
 static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 			      struct multi_pack_index *midx)
 {
+	int fd;
 	struct stat st;
+
 	char *bitmap_name = midx_bitmap_filename(midx);
-	int fd = git_open(bitmap_name);
+	trace2_data_string("midx", the_repository, "path", bitmap_name);
+	fd = git_open(bitmap_name);
 
 	free(bitmap_name);
 
@@ -346,8 +349,10 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 	if (load_bitmap_header(bitmap_git) < 0)
 		goto cleanup;
 
-	if (!hasheq(get_midx_checksum(bitmap_git->midx), bitmap_git->checksum))
+	if (!hasheq(get_midx_checksum(bitmap_git->midx), bitmap_git->checksum)) {
+		error(_("midx and bitmap checksum don't match"));
 		goto cleanup;
+	}
 
 	if (load_midx_revindex(bitmap_git->midx) < 0) {
 		warning(_("multi-pack bitmap is missing required reverse index"));
@@ -374,6 +379,7 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
 		return -1;
 
 	bitmap_name = pack_bitmap_filename(packfile);
+	trace2_data_string("bitmap", the_repository, "path", bitmap_name);
 	fd = git_open(bitmap_name);
 	free(bitmap_name);
 
@@ -509,11 +515,16 @@ static int open_midx_bitmap(struct repository *r,
 static int open_bitmap(struct repository *r,
 		       struct bitmap_index *bitmap_git)
 {
-	assert(!bitmap_git->map);
+	int ret = 0;
 
+	assert(!bitmap_git->map);
+	trace2_region_enter("pack-bitmap", "open_bitmap", r);
 	if (!open_midx_bitmap(r, bitmap_git))
-		return 0;
-	return open_pack_bitmap(r, bitmap_git);
+		goto done;
+	ret = open_pack_bitmap(r, bitmap_git);
+done:
+	trace2_region_leave("pack-bitmap", "open_bitmap", r);
+	return ret;
 }
 
 struct bitmap_index *prepare_bitmap_git(struct repository *r)
diff --git a/repo-settings.c b/repo-settings.c
index b4fbd16cdc..115d96ece3 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -4,10 +4,12 @@
 #include "midx.h"
 
 static void repo_cfg_bool(struct repository *r, const char *key, int *dest,
-			  int def)
+			  int def, int trace)
 {
 	if (repo_config_get_bool(r, key, dest))
 		*dest = def;
+	if (trace)
+		trace2_data_string("config", r, key, *dest ? "true" : "false");
 }
 
 void prepare_repo_settings(struct repository *r)
@@ -29,8 +31,8 @@ void prepare_repo_settings(struct repository *r)
 	r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE;
 
 	/* Booleans config or default, cascades to other settings */
-	repo_cfg_bool(r, "feature.manyfiles", &manyfiles, 0);
-	repo_cfg_bool(r, "feature.experimental", &experimental, 0);
+	repo_cfg_bool(r, "feature.manyfiles", &manyfiles, 0, 0);
+	repo_cfg_bool(r, "feature.experimental", &experimental, 0, 0);
 
 	/* Defaults modified by feature.* */
 	if (experimental) {
@@ -42,13 +44,13 @@ void prepare_repo_settings(struct repository *r)
 	}
 
 	/* Boolean config or default, does not cascade (simple)  */
-	repo_cfg_bool(r, "core.commitgraph", &r->settings.core_commit_graph, 1);
-	repo_cfg_bool(r, "commitgraph.readchangedpaths", &r->settings.commit_graph_read_changed_paths, 1);
-	repo_cfg_bool(r, "gc.writecommitgraph", &r->settings.gc_write_commit_graph, 1);
-	repo_cfg_bool(r, "fetch.writecommitgraph", &r->settings.fetch_write_commit_graph, 0);
-	repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1);
-	repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1);
-	repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0);
+	repo_cfg_bool(r, "core.commitgraph", &r->settings.core_commit_graph, 1, 0);
+	repo_cfg_bool(r, "commitgraph.readchangedpaths", &r->settings.commit_graph_read_changed_paths, 1, 0);
+	repo_cfg_bool(r, "gc.writecommitgraph", &r->settings.gc_write_commit_graph, 1, 0);
+	repo_cfg_bool(r, "fetch.writecommitgraph", &r->settings.fetch_write_commit_graph, 0, 0);
+	repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1, 0);
+	repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1, 1);
+	repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
-- 
2.35.1.582.g270d558070.dirty


  parent reply	other threads:[~2022-06-21 13:34 UTC|newest]

Thread overview: 129+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-24 11:43 [PATCH v1 0/3] trace2 output for bitmap decision path Teng Long
2022-03-24 11:43 ` [PATCH v1 1/3] pack-bitmap.c: use "ret" in "open_midx_bitmap()" Teng Long
2022-03-24 19:11   ` Taylor Blau
2022-03-28  7:59     ` [PATCH v1 1/3] pack-bitmap.c: use "ret" in "open_midx_bitmap() Teng Long
2022-03-30  2:39       ` Taylor Blau
2022-03-24 11:44 ` [PATCH v1 2/3] pack-bitmap.c: add "break" statement in "open_pack_bitmap()" Teng Long
2022-03-24 18:40   ` Junio C Hamano
2022-03-24 19:06     ` Taylor Blau
2022-03-24 19:03   ` Taylor Blau
2022-03-29  2:49     ` Teng Long
2022-03-30  2:55       ` Taylor Blau
2022-03-30  7:32         ` Teng Long Teng Long
2022-03-30 13:17           ` Ævar Arnfjörð Bjarmason
2022-03-24 11:44 ` [PATCH v1 3/3] bitmap: add trace outputs during open "bitmap" file Teng Long
2022-03-24 18:42   ` Junio C Hamano
2022-03-24 13:22 ` [PATCH v1 0/3] trace2 output for bitmap decision path Ævar Arnfjörð Bjarmason
2022-03-29  7:38   ` Teng Long Teng Long
2022-03-29  8:54     ` Ævar Arnfjörð Bjarmason
2022-04-21 13:26 ` [PATCH v2 0/5] trace2 output for bitmap decision path Teng Long
2022-04-21 13:26   ` [PATCH v2 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-05-11 21:31     ` Taylor Blau
2022-04-21 13:26   ` [PATCH v2 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-05-11 21:31     ` Taylor Blau
2022-04-21 13:26   ` [PATCH v2 3/5] pack-bitmap.c: make warnings more detailed when opening bitmap Teng Long
2022-04-21 17:25     ` Taylor Blau
2022-05-06  9:08       ` Teng Long
2022-04-21 13:26   ` [PATCH v2 4/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-04-21 15:51     ` Ævar Arnfjörð Bjarmason
2022-05-06 11:27       ` Teng Long
2022-05-06 11:53       ` Teng Long
2022-04-21 16:32     ` Jeff Hostetler
2022-05-06 12:43       ` Teng Long
2022-05-10 20:47         ` Jeff Hostetler
2022-04-21 13:26   ` [PATCH v2 5/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-04-21 15:41     ` Ævar Arnfjörð Bjarmason
2022-05-06 12:55       ` Teng Long
2022-06-12  7:44   ` [PATCH v3 0/5] trace2 output for bitmap decision path Teng Long
2022-06-12  7:44     ` [PATCH v3 0/5] Teng Long
2022-06-12  7:44     ` [PATCH v3 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-12  7:44     ` [PATCH v3 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-12  7:44     ` [PATCH v3 3/5] pack-bitmap.c: make warnings support i18N when opening bitmap Teng Long
2022-06-12  7:44     ` [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-14  1:15       ` Taylor Blau
2022-06-20 13:17         ` Teng Long
2022-06-12  7:44     ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-06-13 20:59       ` Junio C Hamano
2022-06-20 13:32         ` Teng Long
2022-06-14  1:40       ` Taylor Blau
2022-06-21  6:58         ` Teng Long
2022-06-22 12:51       ` Jeff Hostetler
2022-06-23  9:38         ` Teng Long
2022-06-23 15:14           ` Jeff Hostetler
2022-06-24  8:27             ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" Teng Long
2022-06-21 13:25     ` [PATCH v3 0/5] trace2 output for bitmap decision path Teng Long
2022-06-21 13:25       ` [PATCH v3 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-21 13:25       ` [PATCH v3 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-21 13:25       ` [PATCH v3 3/5] pack-bitmap.c: make warnings support i18N when opening bitmap Teng Long
2022-06-21 13:25       ` [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-21 13:25       ` Teng Long [this message]
2022-06-22 13:04         ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file Jeff Hostetler
2022-06-22 15:12           ` Junio C Hamano
2022-06-28  8:17       ` [PATCH v5 0/5] tr2: avoid to print "interesting" config repeatedly Teng Long
2022-06-28  8:17         ` [PATCH v5 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-28  8:17         ` [PATCH v5 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-28  8:17         ` [PATCH v5 3/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-28 18:04           ` Junio C Hamano
2022-07-05  9:04             ` Teng Long
2022-07-05 18:23               ` Junio C Hamano
2022-06-28  8:17         ` [PATCH v5 4/5] pack-bitmap.c: retrieve missing i18n translations Teng Long
2022-06-28  8:58           ` Ævar Arnfjörð Bjarmason
2022-06-28 17:28             ` Eric Sunshine
2022-07-06 14:19               ` Teng Long
2022-07-06 14:06             ` Teng Long
2022-06-28 18:07           ` Junio C Hamano
2022-07-07 11:59             ` Teng Long
2022-07-07 16:45               ` Junio C Hamano
2022-07-11 11:04                 ` Teng Long
2022-06-28  8:17         ` [PATCH v5 5/5] tr2: avoid to print "interesting" config repeatedly Teng Long
2022-06-28  9:13           ` Ævar Arnfjörð Bjarmason
2022-06-28 18:12             ` Junio C Hamano
2022-07-01 14:31               ` Jeff Hostetler
2022-07-11  4:11                 ` Teng Long
2022-07-11  3:51             ` Teng Long
2022-07-11 12:43         ` [PATCH v6 0/7] trace2: dump scope when print "interesting" config Teng Long
2022-07-11 12:43           ` [PATCH v6 1/7] clean: fixed issues related to text output format Teng Long
2022-07-11 21:08             ` Junio C Hamano
2022-07-13 11:44               ` Teng Long
2022-07-11 12:43           ` [PATCH v6 2/7] pack-bitmap.c: mark more strings for translations Teng Long
2022-07-11 12:43           ` [PATCH v6 3/7] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-07-11 12:44           ` [PATCH v6 4/7] pack-bitmap.c: don't ignore ENOENT silently Teng Long
2022-07-11 14:38             ` Ævar Arnfjörð Bjarmason
2022-07-13 14:14               ` Teng Long
2022-07-11 21:22             ` Junio C Hamano
2022-07-14 15:25               ` Teng Long
2022-07-11 12:44           ` [PATCH v6 5/7] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-07-11 14:53             ` Ævar Arnfjörð Bjarmason
2022-07-15  2:34               ` Teng Long
2022-07-11 12:44           ` [PATCH v6 6/7] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-07-11 12:44           ` [PATCH v6 7/7] tr2: dump names if config exist in multiple scopes Teng Long
2022-07-11 14:40             ` Ævar Arnfjörð Bjarmason
2022-07-11 19:19             ` Jeff Hostetler
2022-07-11 14:59           ` [PATCH v6 0/7] trace2: dump scope when print "interesting" config Ævar Arnfjörð Bjarmason
2022-07-18  8:36             ` Teng Long
2022-07-18 16:45           ` [PATCH v7 " Teng Long
2022-07-18 16:46             ` [PATCH v7 1/7] pack-bitmap.c: fix formatting of error messages Teng Long
2022-07-18 16:46             ` [PATCH v7 2/7] pack-bitmap.c: mark more strings for translations Teng Long
2022-07-18 16:46             ` [PATCH v7 3/7] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-07-18 16:46             ` [PATCH v7 4/7] pack-bitmap.c: do not ignore error when opening a bitmap file Teng Long
2022-07-18 16:46             ` [PATCH v7 5/7] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-07-18 16:46             ` [PATCH v7 6/7] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-07-18 16:46             ` [PATCH v7 7/7] tr2: dump names if config exist in multiple scopes Teng Long
2022-07-18 20:13               ` Jeff Hostetler
2022-07-19  7:40                 ` tenglong.tl
2022-07-19 21:03               ` Junio C Hamano
2022-07-20 12:48                 ` tenglong.tl
2022-07-18 18:57             ` [PATCH v7 0/7] trace2: dump scope when print "interesting" config Junio C Hamano
2022-07-18 19:07               ` Ævar Arnfjörð Bjarmason
2022-07-19 11:26                 ` tenglong.tl
2022-07-19 11:42                   ` Ævar Arnfjörð Bjarmason
2022-07-19 12:34                     ` tenglong.tl
2022-07-21  9:05             ` [PATCH v8 0/6] pack-bitmap.c: optimize error messages tenglong.tl
2022-07-21  9:05               ` [PATCH v8 1/6] pack-bitmap.c: fix formatting of " tenglong.tl
2022-07-21  9:05               ` [PATCH v8 2/6] pack-bitmap.c: mark more strings for translations tenglong.tl
2022-07-21  9:05               ` [PATCH v8 3/6] pack-bitmap.c: rename "idx_name" to "bitmap_name" tenglong.tl
2022-07-21  9:05               ` [PATCH v8 4/6] pack-bitmap.c: do not ignore error when opening a bitmap file tenglong.tl
2022-07-21  9:05               ` [PATCH v8 5/6] pack-bitmap.c: using error() instead of silently returning -1 tenglong.tl
2022-07-21  9:05               ` [PATCH v8 6/6] pack-bitmap.c: continue looping when first MIDX bitmap is found tenglong.tl
2022-07-21 23:01               ` [PATCH v8 0/6] pack-bitmap.c: optimize error messages Junio C Hamano
2022-07-22  6:17                 ` tenglong.tl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8735ae99798a43650acb9a1df7ba2072c7758476.1655817253.git.dyroneteng@gmail.com \
    --to=dyroneteng@gmail.com \
    --cc=avarab@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=tenglong.tl@alibaba-inc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).