Skip to content

Merge requests raid interface#2

Open
LeonidElkin wants to merge 18 commits intomasterfrom
merge-requests-raid-interface
Open

Merge requests raid interface#2
LeonidElkin wants to merge 18 commits intomasterfrom
merge-requests-raid-interface

Conversation

@LeonidElkin
Copy link
Owner

Description

Added implementation of merging sequentional write requests


Implementation:

  • (raid_request_merge.c, raid_request_merge.h) - Implemented interface for the merging requests process
  • (bdev_raid.h, raid5.c) - Integrated into the raid5

Integration tests:

  • (run_tests.sh) - A script that tests for the correctness of operations
  • (test$i/) - Contains fio configuration of the tests

static void
raid_free_merge_info(struct raid_bdev *raid_bdev)
{
if (raid_bdev->merge_info) { raid_clear_ht(raid_bdev); }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

кодстайл

switch (raid_bdev->level) {
case RAID1:
if (raid_bdev->num_base_bdevs % 2 == 1) {
SPDK_WARNLOG("Merging requests is not supported for RAID1 with an odd number of base bdevs\n");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wat? Почему?

raid_bdev->merge_info = NULL;
return 0;
} else {
num_parity_strips = raid_bdev->num_base_bdevs / 2;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У нас же рейд 1, а не рейд 10. В нем "не данные" это raid_bdev->num_base_bdevs - 1

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Бес попутал, исправлю

return -ENOMEM;
}

raid_bdev->merge_info->merge_ht = ht_create();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обработать ошибку

if (merge) {
int ret = raid_merge_info_alloc(raid_bdev);
if (ret) {
free(raid_bdev->base_bdev_info);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему не raid_free_merge_info ?

enum raid_level level;

/* Info required for merging requests */
struct raid_bdev_merge_info *merge_info;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

кодстайл

}

static uint64_t
hash_key(const char *key)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему _key?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так как хешируем ключ. Или вы что-то другое имеете ввиду?

ht_get(ht *table, const char *key)
{
uint64_t hash = hash_key(key);
size_t index = (size_t)(hash & (uint64_t)(table->capacity - 1));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут видимо использован прием, что capacity это степень двойки, но вроде это нигде не проверяется

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В ht.h define стоит, что изначальная длина 16, а так расширяемся в два раза, то и будет всегда степень двойки


index++;
if (index >= table->capacity) {
index = 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут нет бесконечного цикла при заполненной таблице без нужного элемента?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Таблица никогда не бывает полностью заполненной. Иначе очень часто бы встречались с коллизией. Таблица расширяется в два раза, когда достигается половина изначальной вместимости

}
}

if (plength != NULL) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Без указателя на длину же получим некорректную работу, длина не будет уыеличиваться. Тут разве не надо завершить функцию с ошибкой в этом случае?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вспомогательная функция ht_set_entry используется в двух местах: в ht_expand и в ht_set. В первом случае указатель на длину ставится NULL, чтобы её не увеличивать при расширении, во втором передается указатель на длину таблицы, так как это штатная вставка ключа-значения пользователем

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants