Skip to content
Snippets Groups Projects

Journal Synchronized

Merged Ghost User requested to merge journal_synchronized into master

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
159 memset(&j->fl, 0, sizeof(struct flock));
160 j->fl.l_type = F_WRLCK;
161 j->fl.l_whence = SEEK_SET;
162 j->fl.l_start = 0;
163 j->fl.l_len = 0;
164 j->fl.l_pid = getpid();
165
166 /* Attempt to lock. */
167 dbg_journal_verb("journal: locking journal %s\n", j->path);
168 ret = fcntl(j->fd, F_SETLK, &j->fl);
169
170 /* Lock. */
171 if (ret < 0) {
172 struct flock efl;
173 memcpy(&efl, &j->fl, sizeof(struct flock));
174 fcntl(j->fd, F_GETLK, &efl);
  • If the lock is removed before this call, l_type will be set to F_UNLCK and other fields will be unchanged... probably not an issue, but the warning message will be confusing.

  • Author Contributor

    Maybe, but it will lock immediately so...

  • 180 }
    181 j->fl.l_type = F_UNLCK;
    182 dbg_journal("journal: locked journal %s (returned %d)\n", j->path, ret);
    183
    184 /* Read magic bytes. */
    185 dbg_journal("journal: reading magic bytes\n");
    186 const char magic_req[MAGIC_LENGTH] = JOURNAL_MAGIC;
    187 char magic[MAGIC_LENGTH];
    188 if (!sfread(magic, MAGIC_LENGTH, j->fd)) {
    189 dbg_journal_verb("journal: cannot read magic bytes\n");
    190 goto open_file_error;
    191 }
    192 if (memcmp(magic, magic_req, MAGIC_LENGTH) != 0) {
    193 log_server_warning("Journal file '%s' version is too old, "
    194 "it will be flushed.\n", j->path);
    195 fcntl(j->fd, F_SETLK, &j->fl);
    • Is it safe to unlock the file here? If the journal is created in the next few moments, but before some other writer waiting on the lock end up at this place of code, the journal will be recreated multiple times...

    • Author Contributor

      There's not much we can do, since we need to reopen the file anyway (closing it will clear the lock). It will recreate the file and overwrite the previously created. Is it a problem? Probably not, because open()'s in the same process are guarded by a mutex, running several instances of knotd on a same file is going to yield an unpredictable result. It could be solved by having a separate lockfile, but... yes, but not in this patch.

    • :thumbsup: ack

  • 310 qhead_free = (jnode_flags(j, qhead_free) <= JOURNAL_FREE);
    311 if ((j->qhead != j->qtail) && (!qtail_free || !qhead_free)) {
    312 log_server_warning("Recovering journal '%s' metadata "
    313 "after crash.\n",
    314 j->path);
    315 ret = journal_recover(j);
    316 if (ret != KNOT_EOK) {
    317 log_server_error("Journal file '%s' is unrecoverable, "
    318 "metadata corrupted - %s\n",
    319 j->path, knot_strerror(ret));
    320 goto open_file_error;
    321 }
    322 }
    323
    324 /* Save file lock and return. */
    325 j->fl.l_type = F_WRLCK;
  • 166 /* Attempt to lock. */
    167 dbg_journal_verb("journal: locking journal %s\n", j->path);
    168 ret = fcntl(j->fd, F_SETLK, &j->fl);
    169
    170 /* Lock. */
    171 if (ret < 0) {
    172 struct flock efl;
    173 memcpy(&efl, &j->fl, sizeof(struct flock));
    174 fcntl(j->fd, F_GETLK, &efl);
    175 log_server_warning("Journal file '%s' is locked by process "
    176 "PID=%d, waiting for process to "
    177 "release lock.\n",
    178 j->path, efl.l_pid);
    179 ret = fcntl(j->fd, F_SETLKW, &j->fl);
    180 }
    181 j->fl.l_type = F_UNLCK;
  • Mostly code moving. I found some slight issues, otherwise looks good.

  • Please register or sign in to reply
    Loading