@matheusbsbs / matheus-bagliano / commits / 3a60f6a2c4

reftable/stack: register lockfiles during compaction

We do not register any of the locks we acquire when compacting the reftable stack via our lockfiles interfaces. These locks will thus not be released when Git gets killed. Refactor the code to register locks as lockfiles. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Mar 7, 2024 at 14:10 UTC 3a60f6a2c472e8291894308247a8fecb0c76cb51
2 files changed +123 -134
reftable/stack.c
+121 -134
@@ -978,212 +978,199 @@ static int stack_compact_range(struct reftable_stack *st,
978 size_t first, size_t last,
979 struct reftable_log_expiry_config *expiry)
980 {
981 - char **delete_on_success = NULL, **subtable_locks = NULL, **listp = NULL;
982 - struct strbuf temp_tab_file_name = STRBUF_INIT;
981 + struct strbuf tables_list_buf = STRBUF_INIT;
982 + struct strbuf new_table_temp_path = STRBUF_INIT;
983 struct strbuf new_table_name = STRBUF_INIT;
984 - struct strbuf lock_file_name = STRBUF_INIT;
985 - struct strbuf ref_list_contents = STRBUF_INIT;
984 struct strbuf new_table_path = STRBUF_INIT;
987 - size_t i, j, compact_count;
988 - int err = 0;
989 - int have_lock = 0;
990 - int lock_file_fd = -1;
991 - int is_empty_table = 0;
985 + struct strbuf table_name = STRBUF_INIT;
986 + struct lock_file tables_list_lock = LOCK_INIT;
987 + struct lock_file *table_locks = NULL;
988 + int is_empty_table = 0, err = 0;
989 + size_t i;
990
991 if (first > last || (!expiry && first == last)) {
992 err = 0;
993 goto done;
994 }
995
998 - compact_count = last - first + 1;
999 - REFTABLE_CALLOC_ARRAY(delete_on_success, compact_count + 1);
1000 - REFTABLE_CALLOC_ARRAY(subtable_locks, compact_count + 1);
1001 -
996 st->stats.attempts++;
997
1004 - strbuf_reset(&lock_file_name);
1005 - strbuf_addstr(&lock_file_name, st->list_file);
1006 - strbuf_addstr(&lock_file_name, ".lock");
1007 -
1008 - lock_file_fd =
1009 - open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666);
1010 - if (lock_file_fd < 0) {
1011 - if (errno == EEXIST) {
998 + /*
999 + * Hold the lock so that we can read "tables.list" and lock all tables
1000 + * which are part of the user-specified range.
1001 + */
1002 + err = hold_lock_file_for_update(&tables_list_lock, st->list_file,
1003 + LOCK_NO_DEREF);
1004 + if (err < 0) {
1005 + if (errno == EEXIST)
1006 err = 1;
1013 - } else {
1007 + else
1008 err = REFTABLE_IO_ERROR;
1015 - }
1009 goto done;
1010 }
1018 - /* Don't want to write to the lock for now. */
1019 - close(lock_file_fd);
1020 - lock_file_fd = -1;
1011
1022 - have_lock = 1;
1012 err = stack_uptodate(st);
1024 - if (err != 0)
1013 + if (err)
1014 goto done;
1015
1027 - for (i = first, j = 0; i <= last; i++) {
1028 - struct strbuf subtab_file_name = STRBUF_INIT;
1029 - struct strbuf subtab_lock = STRBUF_INIT;
1030 - int sublock_file_fd = -1;
1031 -
1032 - stack_filename(&subtab_file_name, st,
1033 - reader_name(st->readers[i]));
1034 -
1035 - strbuf_reset(&subtab_lock);
1036 - strbuf_addbuf(&subtab_lock, &subtab_file_name);
1037 - strbuf_addstr(&subtab_lock, ".lock");
1016 + /*
1017 + * Lock all tables in the user-provided range. This is the slice of our
1018 + * stack which we'll compact.
1019 + */
1020 + REFTABLE_CALLOC_ARRAY(table_locks, last - first + 1);
1021 + for (i = first; i <= last; i++) {
1022 + stack_filename(&table_name, st, reader_name(st->readers[i]));
1023
1039 - sublock_file_fd = open(subtab_lock.buf,
1040 - O_EXCL | O_CREAT | O_WRONLY, 0666);
1041 - if (sublock_file_fd >= 0) {
1042 - close(sublock_file_fd);
1043 - } else if (sublock_file_fd < 0) {
1044 - if (errno == EEXIST) {
1024 + err = hold_lock_file_for_update(&table_locks[i - first],
1025 + table_name.buf, LOCK_NO_DEREF);
1026 + if (err < 0) {
1027 + if (errno == EEXIST)
1028 err = 1;
1046 - } else {
1029 + else
1030 err = REFTABLE_IO_ERROR;
1048 - }
1031 + goto done;
1032 }
1033
1051 - subtable_locks[j] = subtab_lock.buf;
1052 - delete_on_success[j] = subtab_file_name.buf;
1053 - j++;
1054 -
1055 - if (err != 0)
1034 + /*
1035 + * We need to close the lockfiles as we might otherwise easily
1036 + * run into file descriptor exhaustion when we compress a lot
1037 + * of tables.
1038 + */
1039 + err = close_lock_file_gently(&table_locks[i - first]);
1040 + if (err < 0) {
1041 + err = REFTABLE_IO_ERROR;
1042 goto done;
1043 + }
1044 }
1045
1059 - err = unlink(lock_file_name.buf);
1060 - if (err < 0)
1046 + /*
1047 + * We have locked all tables in our range and can thus release the
1048 + * "tables.list" lock while compacting the locked tables. This allows
1049 + * concurrent updates to the stack to proceed.
1050 + */
1051 + err = rollback_lock_file(&tables_list_lock);
1052 + if (err < 0) {
1053 + err = REFTABLE_IO_ERROR;
1054 goto done;
1062 - have_lock = 0;
1063 -
1064 - err = stack_compact_locked(st, first, last, &temp_tab_file_name,
1065 - expiry);
1066 - /* Compaction + tombstones can create an empty table out of non-empty
1067 - * tables. */
1068 - is_empty_table = (err == REFTABLE_EMPTY_TABLE_ERROR);
1069 - if (is_empty_table) {
1070 - err = 0;
1055 }
1072 - if (err < 0)
1073 - goto done;
1056
1075 - lock_file_fd =
1076 - open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666);
1077 - if (lock_file_fd < 0) {
1078 - if (errno == EEXIST) {
1057 + /*
1058 + * Compact the now-locked tables into a new table. Note that compacting
1059 + * these tables may end up with an empty new table in case tombstones
1060 + * end up cancelling out all refs in that range.
1061 + */
1062 + err = stack_compact_locked(st, first, last, &new_table_temp_path, expiry);
1063 + if (err < 0) {
1064 + if (err != REFTABLE_EMPTY_TABLE_ERROR)
1065 + goto done;
1066 + is_empty_table = 1;
1067 + }
1068 +
1069 + /*
1070 + * Now that we have written the new, compacted table we need to re-lock
1071 + * "tables.list". We'll then replace the compacted range of tables with
1072 + * the new table.
1073 + */
1074 + err = hold_lock_file_for_update(&tables_list_lock, st->list_file,
1075 + LOCK_NO_DEREF);
1076 + if (err < 0) {
1077 + if (errno == EEXIST)
1078 err = 1;
1080 - } else {
1079 + else
1080 err = REFTABLE_IO_ERROR;
1082 - }
1081 goto done;
1082 }
1085 - have_lock = 1;
1083 +
1084 if (st->config.default_permissions) {
1087 - if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) {
1085 + if (chmod(get_lock_file_path(&tables_list_lock),
1086 + st->config.default_permissions) < 0) {
1087 err = REFTABLE_IO_ERROR;
1088 goto done;
1089 }
1090 }
1091
1093 - format_name(&new_table_name, st->readers[first]->min_update_index,
1094 - st->readers[last]->max_update_index);
1095 - strbuf_addstr(&new_table_name, ".ref");
1096 -
1097 - stack_filename(&new_table_path, st, new_table_name.buf);
1098 -
1092 + /*
1093 + * If the resulting compacted table is not empty, then we need to move
1094 + * it into place now.
1095 + */
1096 if (!is_empty_table) {
1100 - /* retry? */
1101 - err = rename(temp_tab_file_name.buf, new_table_path.buf);
1097 + format_name(&new_table_name, st->readers[first]->min_update_index,
1098 + st->readers[last]->max_update_index);
1099 + strbuf_addstr(&new_table_name, ".ref");
1100 + stack_filename(&new_table_path, st, new_table_name.buf);
1101 +
1102 + err = rename(new_table_temp_path.buf, new_table_path.buf);
1103 if (err < 0) {
1104 err = REFTABLE_IO_ERROR;
1105 goto done;
1106 }
1107 }
1108
1108 - for (i = 0; i < first; i++) {
1109 - strbuf_addstr(&ref_list_contents, st->readers[i]->name);
1110 - strbuf_addstr(&ref_list_contents, "\n");
1111 - }
1112 - if (!is_empty_table) {
1113 - strbuf_addbuf(&ref_list_contents, &new_table_name);
1114 - strbuf_addstr(&ref_list_contents, "\n");
1115 - }
1116 - for (i = last + 1; i < st->merged->stack_len; i++) {
1117 - strbuf_addstr(&ref_list_contents, st->readers[i]->name);
1118 - strbuf_addstr(&ref_list_contents, "\n");
1119 - }
1120 -
1121 - err = write_in_full(lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
1122 - if (err < 0) {
1123 - err = REFTABLE_IO_ERROR;
1124 - unlink(new_table_path.buf);
1125 - goto done;
1126 - }
1127 -
1128 - err = fsync_component(FSYNC_COMPONENT_REFERENCE, lock_file_fd);
1109 + /*
1110 + * Write the new "tables.list" contents with the compacted table we
1111 + * have just written. In case the compacted table became empty we
1112 + * simply skip writing it.
1113 + */
1114 + for (i = 0; i < first; i++)
1115 + strbuf_addf(&tables_list_buf, "%s\n", st->readers[i]->name);
1116 + if (!is_empty_table)
1117 + strbuf_addf(&tables_list_buf, "%s\n", new_table_name.buf);
1118 + for (i = last + 1; i < st->merged->stack_len; i++)
1119 + strbuf_addf(&tables_list_buf, "%s\n", st->readers[i]->name);
1120 +
1121 + err = write_in_full(get_lock_file_fd(&tables_list_lock),
1122 + tables_list_buf.buf, tables_list_buf.len);
1123 if (err < 0) {
1124 err = REFTABLE_IO_ERROR;
1125 unlink(new_table_path.buf);
1126 goto done;
1127 }
1128
1135 - err = close(lock_file_fd);
1136 - lock_file_fd = -1;
1129 + err = fsync_component(FSYNC_COMPONENT_REFERENCE, get_lock_file_fd(&tables_list_lock));
1130 if (err < 0) {
1131 err = REFTABLE_IO_ERROR;
1132 unlink(new_table_path.buf);
1133 goto done;
1134 }
1135
1143 - err = rename(lock_file_name.buf, st->list_file);
1136 + err = commit_lock_file(&tables_list_lock);
1137 if (err < 0) {
1138 err = REFTABLE_IO_ERROR;
1139 unlink(new_table_path.buf);
1140 goto done;
1141 }
1149 - have_lock = 0;
1142
1151 - /* Reload the stack before deleting. On windows, we can only delete the
1152 - files after we closed them.
1153 - */
1143 + /*
1144 + * Reload the stack before deleting the compacted tables. We can only
1145 + * delete the files after we closed them on Windows, so this needs to
1146 + * happen first.
1147 + */
1148 err = reftable_stack_reload_maybe_reuse(st, first < last);
1149 + if (err < 0)
1150 + goto done;
1151
1156 - listp = delete_on_success;
1157 - while (*listp) {
1158 - if (strcmp(*listp, new_table_path.buf)) {
1159 - unlink(*listp);
1160 - }
1161 - listp++;
1152 + /*
1153 + * Delete the old tables. They may still be in use by concurrent
1154 + * readers, so it is expected that unlinking tables may fail.
1155 + */
1156 + for (i = first; i <= last; i++) {
1157 + struct lock_file *table_lock = &table_locks[i - first];
1158 + char *table_path = get_locked_file_path(table_lock);
1159 + unlink(table_path);
1160 + free(table_path);
1161 }
1162
1163 done:
1165 - free_names(delete_on_success);
1164 + rollback_lock_file(&tables_list_lock);
1165 + for (i = first; table_locks && i <= last; i++)
1166 + rollback_lock_file(&table_locks[i - first]);
1167 + reftable_free(table_locks);
1168
1167 - if (subtable_locks) {
1168 - listp = subtable_locks;
1169 - while (*listp) {
1170 - unlink(*listp);
1171 - listp++;
1172 - }
1173 - free_names(subtable_locks);
1174 - }
1175 - if (lock_file_fd >= 0) {
1176 - close(lock_file_fd);
1177 - lock_file_fd = -1;
1178 - }
1179 - if (have_lock) {
1180 - unlink(lock_file_name.buf);
1181 - }
1169 strbuf_release(&new_table_name);
1170 strbuf_release(&new_table_path);
1184 - strbuf_release(&ref_list_contents);
1185 - strbuf_release(&temp_tab_file_name);
1186 - strbuf_release(&lock_file_name);
1171 + strbuf_release(&new_table_temp_path);
1172 + strbuf_release(&tables_list_buf);
1173 + strbuf_release(&table_name);
1174 return err;
1175 }
1176
reftable/system.h
+2
@@ -12,7 +12,9 @@ https://developers.google.com/open-source/licenses/bsd
12 /* This header glues the reftable library to the rest of Git */
13
14 #include "git-compat-util.h"
15 +#include "lockfile.h"
16 #include "strbuf.h"
17 +#include "tempfile.h"
18 #include "hash-ll.h" /* hash ID, sizes.*/
19 #include "dir.h" /* remove_dir_recursively, for tests.*/
20