Fix garbage collection not working properly and add periodic jemalloc purge

[imilinovic]

Fix garbage collection not working properly, add periodic jemalloc memory purge

[master < Epic] PR

• Check, and update documentation if necessary
• Write E2E tests
• Compare the benchmarking results between the master branch and the Epic branch
• Provide the full content or a guide for the final git message

[master < Task] PR

• Check, and update documentation if necessary
• Provide the full content or a guide for the final git message

This fixes Issue1220

To keep docs changelog up to date, one more thing to do:

• Write a release note here, including added/changed clauses
• Tag someone from docs team in the comments

[antepusic]

Good work! I’ve left a few comments, answer appreciated 🙏

[as51340]

I think this is good enough. Could we add some test which will check that memory is actually reclaimed after purging it?

[antoniofilipovic]

Seems fine to me. The only option for me that seems possible to add a test is a stress test where we do a bunch of creates and wait for the GC cycle to kick in, so we assert that memory after gc should kick in is lower than before. Check if that is possible, and put a small gc cycle of 5 seconds, and in the meantime, you can do one FOREACH transaction. That is my idea for the test, otherwise, I am not sure

[Josipmrden]

I'm a bit more for having this under one flag only, and not displaying to the user the info about jemalloc. Couldn't hurt since now we have both cycle seconds set to 30 seconds. I'd also add @Ignition to review this before merging.

[Josipmrden]

PreparedQuery PrepareFreeMemoryQuery(ParsedQuery parsed_query, bool in_explicit_transaction, CurrentDB &current_db) {
  if (in_explicit_transaction) {
    throw FreeMemoryModificationInMulticommandTxException();
  }

  MG_ASSERT(current_db.db_acc_, "Free Memory query expects a current DB");
  storage::Storage *storage = current_db.db_acc_->get()->storage();

  if (storage->GetStorageMode() == storage::StorageMode::ON_DISK_TRANSACTIONAL) {
    throw FreeMemoryDisabledOnDiskStorage();
  }

  return PreparedQuery{{},
                       std::move(parsed_query.required_privileges),
                       [storage](AnyStream *stream, std::optional<int> n) -> std::optional<QueryHandlerResult> {
                         storage->FreeMemory();
                         memory::PurgeUnusedMemory();
                         return QueryHandlerResult::COMMIT;
                       },
                       RWType::NONE};
}

this is the code from the interpreter. After doing free memory, it automatically purges it so it's returned back to the process. I would then assume that we need to do both actions with one flag. User should not be concerned about what jemalloc is, and if we ever need to debug it separately, we can do so later

[antoniofilipovic]

@imilinovic @Josipmrden I am more than okay to have two separate threads, one for GC for our internal stuff, one for jemalloc. I think users already are impacted by jemalloc via vm.max_map_count and they can read about that already there, so it is one more config that they can set up. Also if jemalloc purge blocks the whole thread, I think it will be too slow to do gc cycles and it might be an issue later on.

[Josipmrden]

@imilinovic @Josipmrden I am more than okay to have two separate threads, one for GC for our internal stuff, one for jemalloc. I think users already are impacted by jemalloc via vm.max_map_count and they can read about that already there, so it is one more config that they can set up. Also if jemalloc purge blocks the whole thread, I think it will be too slow to do gc cycles and it might be an issue later on.

Please discuss with @gitbuda about the feature before we merge this

[as51340]

💪

[antoniofilipovic]

💪

[imilinovic]

More proactive GC deletion moved to #1508 because I found a bug in it. More info in the PR.