`AllocateZone` might not find all possible open zones

[skyzh]

I've been reading through the ZenFS source code, and came across this line: https://github.com/westerndigitalcorporation/zenfs/blob/master/fs/zbd_zenfs.cc#L518

  for (const auto z : io_zones) {
    if ((!z->open_for_write_) && (z->used_capacity_ > 0) && !z->IsFull()) {  // <- L518
      unsigned int diff = GetLifeTimeDiff(z->lifetime_, file_lifetime);
      if (diff <= best_diff) {
        allocated_zone = z;
        best_diff = diff;
      }
    }
  }

z->used_capacity_ > 0 seems not necessary in this case. I believe it is possible that there could be a zone where

1. no file is occupying this zone
2. used_capacity > 0, which means all files are deleted (will explain)
3. not full

It is true that ZenFS will reset all unused zone at https://github.com/westerndigitalcorporation/zenfs/blob/master/fs/zbd_zenfs.cc#L488

    if (!z->IsUsed()) {
      if (!z->IsFull()) active_io_zones_--;
      s = z->Reset();
      if (!s.ok()) {
        Debug(logger_, "Failed resetting zone !");
      }
      continue;
    }

And here is one possible execution order that makes this assumption fail.

thread 1	thread 2
	L488 reset unused zones
deleting a file on Zone X, which makes used_capacity of X = 0	...
	L518 executes and missed this "open zone"

I'm wondering if it is necessary to ensure z->used_capacity_ > 0. Could we remove this condition?

Thanks in advance!

[yhr]

Yeah, there is a (small, i believe) possibility that all files that have been previously allocated to a zone have been deleted. I'm planning to revisit the allocator code in the coming weeks anyway (primarily to move resets & finishes to a background thread) and i'll take this feedback into account. Thanks! We can keep this issue open for now.

[yhr]

The allocator has been (almost completely) reworked as part of https://github.com/westerndigitalcorporation/zenfs/pull/114 , so closing this. If you find any issues with the new implementation, please create a new issue. Thanks!