Skip to content

Conversation

@decster
Copy link
Contributor

@decster decster commented Jun 3, 2020

After adding meta, BE now can create MemTablet, and put it into TabletManager, but MemTablet is not compatible with Tablet, a lot of code may break. This CL contains the following changes:

When TabletManager::get_tablet is called, only return Tablet, if the underlying tablet is MemTablet, return an error, this keeps the initial code changes small.

For methods/functionalities Tablet&MemTablet both have, refactor/extract them to BaseTablet, and change the usage to TabletManager::get_base_tablet, and it will gradually remove errors introduced by step1, this is a long going process.

Resolves #3669

@morningman morningman added area/storage/in-memory Issues or PRs related to the memory storage engine kind/refactor Issues or PRs to refactor code labels Jun 5, 2020
@morningman
Copy link
Contributor

Hi @decster , in your comment, you said When TabletManager::get_tablet is called, only return Tablet, if the underlying tablet is MemTablet, return an error, this keeps the initial code changes small.

But i didn't see the code change related to it?

@decster
Copy link
Contributor Author

decster commented Jun 8, 2020

Hi @decster , in your comment, you said When TabletManager::get_tablet is called, only return Tablet, if the underlying tablet is MemTablet, return an error, this keeps the initial code changes small.

But i didn't see the code change related to it?

the related change is in tablet_manager TabletManager::_get_tablet_unlocked

    if (ret->is_memory()) {
        LOG(FATAL) << "_get_tablet_unlocked get MemTablet";
        return TabletSharedPtr();
    }

@morningman
Copy link
Contributor

Hi @decster , in your comment, you said When TabletManager::get_tablet is called, only return Tablet, if the underlying tablet is MemTablet, return an error, this keeps the initial code changes small.
But i didn't see the code change related to it?

the related change is in tablet_manager TabletManager::_get_tablet_unlocked

    if (ret->is_memory()) {
        LOG(FATAL) << "_get_tablet_unlocked get MemTablet";
        return TabletSharedPtr();
    }

OK, I see~

}

if (base_tablet->is_memory() || existed_tablet->is_memory()) {
LOG(WARNING) << "add the same tablet twice! tablet_id=" << tablet_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the comment

@chaoyli chaoyli merged commit ca96ea3 into apache:master Jun 18, 2020
chaoyli pushed a commit that referenced this pull request Jun 23, 2020
chaoyli added a commit that referenced this pull request Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/storage/in-memory Issues or PRs related to the memory storage engine kind/refactor Issues or PRs to refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Memory Engine] MemTablet creation and compatibility handling in BE

3 participants