cache status get #1535
Merged
cache status get #1535
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
achiurizo
approved these changes
May 28, 2020
Comment on lines
41
to
44
| ets = Keyword.fetch!(opts, :ets) | ||
| :ok = event_bus.subscribe({:root_chain, "ethereum_new_height"}, link: true) | ||
| state = ets | ||
| {:ok, eth_block_number} = EthereumHeight.get() |
There was a problem hiding this comment.
Why the additional assignment here?
Suggested change
| ets = Keyword.fetch!(opts, :ets) | |
| :ok = event_bus.subscribe({:root_chain, "ethereum_new_height"}, link: true) | |
| state = ets | |
| {:ok, eth_block_number} = EthereumHeight.get() | |
| state = Keyword.fetch!(opts, :ets) | |
| :ok = event_bus.subscribe({:root_chain, "ethereum_new_height"}, link: true) | |
| {:ok, eth_block_number} = EthereumHeight.get() |
Comment on lines
+58
to
+65
| defp key() do | ||
| :status | ||
| end |
There was a problem hiding this comment.
Suggested change
| defp key() do | |
| :status | |
| end | |
| defp key(), do: :status |
| contract_addr = contract_map_from_hex(contracts) | ||
|
|
||
| mined_child_block_number = RootChain.get_mined_child_block() | ||
|
|
There was a problem hiding this comment.
why the newlines? Is this the formatter?
Comment on lines
+39
to
+41
| def events_bucket() do | ||
| @events_bucket | ||
| end |
There was a problem hiding this comment.
Suggested change
| def events_bucket() do | |
| @events_bucket | |
| end | |
| def events_bucket(), do: @events_bucket |
There was a problem hiding this comment.
you're here for the free commits aren't you
achiurizo
reviewed
May 28, 2020
Comment on lines
95
to
101
| ''' | ||
| Checks geth syncing status, errors are treated as not synced. | ||
| Returns: | ||
| * false - geth is synced | ||
| * true - geth is still syncing. | ||
| ''' | ||
|
|
There was a problem hiding this comment.
Suggested change
| ''' | |
| Checks geth syncing status, errors are treated as not synced. | |
| Returns: | |
| * false - geth is synced | |
| * true - geth is still syncing. | |
| ''' | |
| # Checks geth syncing status, errors are treated as not synced. | |
| # Returns: | |
| # * false - geth is synced | |
| # * true - geth is still syncing. |
mederic-p
reviewed
May 28, 2020
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| defmodule OMG.Watcher.API.StatusCache do |
There was a problem hiding this comment.
Can we have a few tests to cover the new functions?
T-Dnzt
approved these changes
May 28, 2020
InoMurko
force-pushed
the
inomurko/16-sec-status
branch
from
1ea35a6
to
9aabcb8
Compare
InoMurko
force-pushed
the
inomurko/16-sec-status
branch
from
9aabcb8
to
b77df38
Compare
mederic-p
approved these changes
May 28, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Protecting status.get from collapsing
Overview
Putting an ETS table in front of our internals.
Changes
Our underlying mechanisms can change state only on Ethereum block issuance interval (changes are in ethLogs). So pulling it more then once (for one client) per 15 seconds doesn't make any difference. But it buts lots of pressure on watchers internals.
Testing
mix test