Skip to content

Warm reboot: Add support for bool Select::hasCachedSelectable()#214

Merged
lguohan merged 3 commits intosonic-net:masterfrom
jipanyang:hasCachedSelectable
Aug 2, 2018
Merged

Warm reboot: Add support for bool Select::hasCachedSelectable()#214
lguohan merged 3 commits intosonic-net:masterfrom
jipanyang:hasCachedSelectable

Conversation

@jipanyang
Copy link
Copy Markdown
Contributor

Signed-off-by: Jipan Yang [email protected]

For warm restart state restore, there is need to check if all cached selectables have been processed.

@pavel-shirshov
Copy link
Copy Markdown
Contributor

hasCachedSelectable() is a misleading name for me.
Probably isQueueNonEmpty() is better name? I'd better export Boolean status for empty Queue.

What do you think?

@jipanyang
Copy link
Copy Markdown
Contributor Author

@pavel-shirshov I don't have strong opinion as to the function name. Actually it took me several iterations to just get current name.
Since std::set<Selectable *, Select::cmp> m_ready; is a priority queue implementation(?), I guess
isQueueNonEmpty() is a better name.

@pavel-shirshov
Copy link
Copy Markdown
Contributor

Yes, m_queue is a sort of priority queue

@jipanyang jipanyang changed the title Add support for bool Select::hasCachedSelectable() Warm reboot: Add support for bool Select::hasCachedSelectable() Aug 1, 2018
}

};
bool Select::isQueueNonEmpty()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isQueueNonEmpty [](start = 13, length = 15)

Prefer positive adj.
isQueueEmpty

Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comment

Signed-off-by: Jipan Yang <[email protected]>
@lguohan lguohan merged commit c5d5434 into sonic-net:master Aug 2, 2018
@jipanyang jipanyang deleted the hasCachedSelectable branch February 3, 2019 01:42
@qiluo-msft
Copy link
Copy Markdown
Contributor

@jipanyang Is this new function used somewhere? I think it just expose some internal implementation details, and the implementation will possible be changed in near future. Why it is useful? Can we remove it?

qiluo-msft added a commit to qiluo-msft/sonic-swss-common that referenced this pull request Mar 10, 2019
qiluo-msft added a commit to qiluo-msft/sonic-swss-common that referenced this pull request Apr 25, 2019
prgeor added a commit to prgeor/sonic-swss-common that referenced this pull request Feb 27, 2025
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.

4 participants