Skip to content

Endpoint and relation to track downsample datapoints.#1

Open
Gooner91 wants to merge 7 commits intomasterfrom
downsampled-time-series-data-end-point
Open

Endpoint and relation to track downsample datapoints.#1
Gooner91 wants to merge 7 commits intomasterfrom
downsampled-time-series-data-end-point

Conversation

@Gooner91
Copy link
Copy Markdown
Owner

@Gooner91 Gooner91 commented Aug 22, 2021

Summary

Changes in this pull request, introduce a new endpoint to request persisted downsampled datapoints.

Description

Motivation
This was my first time working with downsampling or analysis of time series data. After doing some R&D on how to achieve it, I came across the rollups gem (you may also see me using this gem in a couple of commits to get an idea). This gem helped, me with my design of the solution.

Solution
For the solution, I followed the pattern I observed in the rollups gem and came up with a model to store the downsampled data. The reason to come up with a separate model was to persist the downsampled data (in background or via cron job) and avoid large computations while processing an http request. Also, having a model gives us an opportunity to analyze historical down sampled data.

Sample data set
For this solution I have created a seed file to generate sample datapoints , 10000 per channel which can be executed using:

rails db:seed VERSION=001_sample_data_points.rb
This data set is created with the assumption that the timeseries data has been in coming at a frequency of 1 minute.

Implementation of downsampling_datapoints model

  • the new model I came up with to store the downsampled data is associated with channel. The reason being, we may want to see downsampled data against each channel.

Implementation of downsampling logic

  • For the implementation I have written a service which will downsample all the datapoints to a frequency of 5 minutes
  • Once, the data is downsampled it is inserted in bulk inside the db (to avoid multiple insertion queires)
  • This service is added in the crontab via rake task, the idea is that downsampling will occur periodically on newly added datapoints

Implementation of api endpoint

  • a new endpoint is added which will return paginated downsampled data in json api format
  • have also added a couple of specs against the end point

Improvements

Due to the time constraint I was able to come up with this PR, but there are some improvements that I can think of, would have implemented if there was more time

  • I can see some room for improvement in the downsampling logic, I came up with. There is room for some code clean up a well.
  • Better and extensive test coverage, especially of the service that generates the downsampling
  • General code clean up in the overall approach
  • improving quality of sample data set

respond_with data_points
end

def downsampled
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/EmptyMethod: Put empty method definitions on a single line.

end

def downsampled

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/EmptyLinesAroundMethodBody: Extra empty line detected at method body beginning.

after_commit :broadcast
attr_accessor :skip_broadcast

after_commit :broadcast, unless: Proc.new{ |data_point| data_point.skip_broadcast }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/Proc: Use proc instead of Proc.new.

after_commit :broadcast
attr_accessor :skip_broadcast

after_commit :broadcast, unless: Proc.new{ |data_point| data_point.skip_broadcast }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/SpaceBeforeBlockBraces: Space missing to the left of {.

@@ -0,0 +1,5 @@
class DownsampledDatapoint < ApplicationRecord
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.

end

end
end No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/TrailingEmptyLines: Final newline missing.

@@ -0,0 +1,5 @@
FactoryBot.define do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.

@@ -0,0 +1,5 @@
FactoryBot.define do
factory :downsampled_datapoint do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ [rubocop] reported by reviewdog 🐶
Lint/EmptyBlock: Empty block detected.

@@ -0,0 +1,5 @@
FactoryBot.define do
factory :downsampled_datapoint do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/TrailingWhitespace: Trailing whitespace detected.

@@ -0,0 +1,5 @@
require 'rails_helper'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.

def down_sampled
down_sampled_data = DownsampledDatapoint.paginate(page: params[:page],
per_page: 20)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/TrailingWhitespace: Trailing whitespace detected.

per_page: 20)

render json: DataPoints::DownSampledDataSerializer
.new(down_sampled_data)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/MultilineMethodCallIndentation: Use 2 (not 15) spaces for indenting an expression spanning multiple lines.


render json: DataPoints::DownSampledDataSerializer
.new(down_sampled_data)
.serializable_hash
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/MultilineMethodCallIndentation: Use 2 (not 15) spaces for indenting an expression spanning multiple lines.

.new(down_sampled_data)
.serializable_hash
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body end.

class DownsampledDatapoint < ApplicationRecord
belongs_to :channel

enum interval: %w[5m 10m 15m]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Rails/EnumHash: Enum defined as an array found in interval enum declaration. Use hash syntax instead.

let!(:down_sampled_data){ create_list(:downsampled_datapoint,
100,
channel: channel,
value: rand(1.0..999.9)) }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] RSpec/EmptyLineAfterFinalLet: Add an empty line after the last let!.

let!(:down_sampled_data){ create_list(:downsampled_datapoint,
100,
channel: channel,
value: rand(1.0..999.9)) }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Layout/BlockEndNewline: Expression at 11, 66 should be on its own line.

it 'returns appropriate attributes' do
get :down_sampled, format: :json, params: { page: 1 }
parsed_response = JSON.parse(response.body)['data']
expect(parsed_response.first["attributes"].keys)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

@@ -0,0 +1,7 @@
FactoryBot.define do
factory :downsampled_datapoint do
interval { '5m' }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/BlockDelimiters: Prefer do...end over {...} for procedural blocks.

FactoryBot.define do
factory :downsampled_datapoint do
interval { '5m' }
time_interval { Time.now + 5.minutes }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Rails/TimeZone: Do not use Time.now without zone. Use one of Time.zone.now, Time.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.xmlschema, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.httpdate, Time.now.to_i, Time.now.to_f instead.

@time_lower_bound = set_time_lower_bound
end

def call
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
Metrics/AbcSize: Assignment Branch Condition size for call is too high. [<7, 16, 5> 18.17/17]

value: average.to_f,
time_interval: time_upper_bound,
created_at: Time.now.utc,
updated_at: Time.now.utc, }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/TrailingCommaInHashLiteral: Avoid comma after the last item of a hash.

describe Api::DataPointsController, type: :controller do
let(:device) { create(:device) }
let(:channel) { create(:channel, device: device) }
let!(:down_sampled_data) do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
RSpec/LetSetup: Do not use let! to setup objects not referenced in tests.

@Gooner91 Gooner91 changed the title Added seed file to add dummy data_points for each device. Endpoint and relation to track downsample datapoints. Aug 24, 2021
@Gooner91 Gooner91 self-assigned this Aug 24, 2021
end

task down_sample: :environment do
DataPoints::Rollup.new().call
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[rubocop] reported by reviewdog 🐶
[Correctable] Style/MethodCallWithoutArgsParentheses: Do not use parentheses for method calls with no arguments.

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.

1 participant