closes #5383
authorRadhika Chippada <radhika@curoverse.com>
Wed, 18 Mar 2015 01:08:31 +0000 (21:08 -0400)
committerRadhika Chippada <radhika@curoverse.com>
Wed, 18 Mar 2015 01:08:31 +0000 (21:08 -0400)
Merge branch '5383-api-db-current-time'

12 files changed:
services/api/app/controllers/arvados/v1/jobs_controller.rb
services/api/app/controllers/arvados/v1/nodes_controller.rb
services/api/app/controllers/arvados/v1/schema_controller.rb
services/api/app/models/arvados_model.rb
services/api/app/models/blob.rb
services/api/app/models/job.rb
services/api/app/models/keep_disk.rb
services/api/app/models/log.rb
services/api/app/models/node.rb
services/api/config/initializers/db_current_time.rb [new file with mode: 0644]
services/api/lib/db_current_time.rb [new file with mode: 0644]
services/api/test/unit/arvados_model_test.rb

index bd6cbd0df6b9d63b92d2b2a197da07de8ef707ac..ce8a05cba26888d649f6fc28cf7de8de55ee6407 100644 (file)
@@ -5,6 +5,8 @@ class Arvados::V1::JobsController < ApplicationController
   skip_before_filter :find_object_by_uuid, :only => [:queue, :queue_size]
   skip_before_filter :render_404_if_no_object, :only => [:queue, :queue_size]
 
+  include DbCurrentTime
+
   def create
     [:repository, :script, :script_version, :script_parameters].each do |r|
       if !resource_attrs[r]
@@ -122,8 +124,9 @@ class Arvados::V1::JobsController < ApplicationController
       while not @job.started_at
         # send a summary (job queue + available nodes) to the client
         # every few seconds while waiting for the job to start
-        last_ack_at ||= Time.now - Q_UPDATE_INTERVAL - 1
-        if Time.now - last_ack_at >= Q_UPDATE_INTERVAL
+        current_time = db_current_time
+        last_ack_at ||= current_time - Q_UPDATE_INTERVAL - 1
+        if current_time - last_ack_at >= Q_UPDATE_INTERVAL
           nodes_in_state = {idle: 0, alloc: 0}
           ActiveRecord::Base.uncached do
             Node.where('hostname is not ?', nil).collect do |n|
@@ -139,13 +142,13 @@ class Arvados::V1::JobsController < ApplicationController
             break if j.uuid == @job.uuid
             n_queued_before_me += 1
           end
-          yield "#{Time.now}" \
+          yield "#{db_current_time}" \
             " job #{@job.uuid}" \
             " queue_position #{n_queued_before_me}" \
             " queue_size #{job_queue.size}" \
             " nodes_idle #{nodes_in_state[:idle]}" \
             " nodes_alloc #{nodes_in_state[:alloc]}\n"
-          last_ack_at = Time.now
+          last_ack_at = db_current_time
         end
         sleep 3
         ActiveRecord::Base.uncached do
index 44197a054feba0176091b6e7e6aabcb640b84f11..4ab5695a2f1c607f22bec9bd1948d575c73fd29e 100644 (file)
@@ -3,6 +3,8 @@ class Arvados::V1::NodesController < ApplicationController
   skip_before_filter :find_object_by_uuid, :only => :ping
   skip_before_filter :render_404_if_no_object, :only => :ping
 
+  include DbCurrentTime
+
   def update
     if resource_attrs[:job_uuid]
       @object.job_readable = readable_job_uuids(resource_attrs[:job_uuid]).any?
@@ -41,7 +43,7 @@ class Arvados::V1::NodesController < ApplicationController
     if !current_user.andand.is_admin && current_user.andand.is_active
       # active non-admin users can list nodes that are (or were
       # recently) working
-      @objects = model_class.where('last_ping_at >= ?', Time.now - 1.hours)
+      @objects = model_class.where('last_ping_at >= ?', db_current_time - 1.hours)
     end
     super
     job_uuids = @objects.map { |n| n[:job_uuid] }.compact
index 8dc90e1df95cce87457642734c108fbea6500bfe..dcc9c639793eebe17f39a21f05990c81512431ce 100644 (file)
@@ -9,6 +9,8 @@ class Arvados::V1::SchemaController < ApplicationController
   skip_before_filter :render_404_if_no_object
   skip_before_filter :require_auth_scope
 
+  include DbCurrentTime
+
   def index
     expires_in 24.hours, public: true
     discovery = Rails.cache.fetch 'arvados_v1_rest_discovery' do
@@ -21,7 +23,7 @@ class Arvados::V1::SchemaController < ApplicationController
         version: "v1",
         revision: "20131114",
         source_version: (Rails.application.config.source_version ? Rails.application.config.source_version : "No version information available") + (Rails.application.config.local_modified ? Rails.application.config.local_modified.to_s : ''),
-        generatedAt: Time.now.iso8601,
+        generatedAt: db_current_time.iso8601,
         title: "Arvados API",
         description: "The API to interact with Arvados.",
         documentationLink: "http://doc.arvados.org/api/index.html",
index cf0aba9721dffc82dca1ca3d7860244bbfc0a076..1fe58088483fad98e34531391bd2b21a5bf91deb 100644 (file)
@@ -4,6 +4,7 @@ class ArvadosModel < ActiveRecord::Base
   self.abstract_class = true
 
   include CurrentApiClient      # current_user, current_api_client, etc.
+  include DbCurrentTime
 
   attr_protected :created_at
   attr_protected :modified_by_user_uuid
@@ -358,9 +359,10 @@ class ArvadosModel < ActiveRecord::Base
   end
 
   def update_modified_by_fields
-    self.updated_at = Time.now
+    current_time = db_current_time
+    self.updated_at = current_time
     self.owner_uuid ||= current_default_owner if self.respond_to? :owner_uuid=
-    self.modified_at = Time.now
+    self.modified_at = current_time
     self.modified_by_user_uuid = current_user ? current_user.uuid : nil
     self.modified_by_client_uuid = current_api_client ? current_api_client.uuid : nil
     true
index 7d16048bf81a29d8a806caa5904e2870a42fbf6e..799279d0400f31c2cf54feb503a1f48401e96321 100644 (file)
@@ -1,4 +1,5 @@
 class Blob
+  extend DbCurrentTime
 
   def initialize locator
     @locator = locator
@@ -43,7 +44,7 @@ class Blob
       end
       timestamp = opts[:expire]
     else
-      timestamp = Time.now.to_i + (opts[:ttl] || 1209600)
+      timestamp = db_current_time.to_i + (opts[:ttl] || 1209600)
     end
     timestamp_hex = timestamp.to_s(16)
     # => "53163cb4"
@@ -90,7 +91,7 @@ class Blob
     if !timestamp.match /^[\da-f]+$/
       raise Blob::InvalidSignatureError.new 'Timestamp is not a base16 number.'
     end
-    if timestamp.to_i(16) < Time.now.to_i
+    if timestamp.to_i(16) < db_current_time.to_i
       raise Blob::InvalidSignatureError.new 'Signature expiry time has passed.'
     end
 
index 934c7fa36a33dac791a4fcbf3b4cbd3d03096560..01df069f32f90ae2cc4dd7955c76b6f7b9c572c0 100644 (file)
@@ -63,7 +63,7 @@ class Job < ArvadosModel
            ]
 
   def assert_finished
-    update_attributes(finished_at: finished_at || Time.now,
+    update_attributes(finished_at: finished_at || db_current_time,
                       success: success.nil? ? false : success,
                       running: false)
   end
@@ -239,7 +239,7 @@ class Job < ArvadosModel
       # Ensure cancelled_at cannot be set to arbitrary non-now times,
       # or changed once it is set.
       if self.cancelled_at and not self.cancelled_at_was
-        self.cancelled_at = Time.now
+        self.cancelled_at = db_current_time
         self.cancelled_by_user_uuid = current_user.uuid
         self.cancelled_by_client_uuid = current_api_client.andand.uuid
         @need_crunch_dispatch_trigger = true
@@ -265,11 +265,11 @@ class Job < ArvadosModel
 
     case state
     when Running
-      self.started_at ||= Time.now
+      self.started_at ||= db_current_time
     when Failed, Complete
-      self.finished_at ||= Time.now
+      self.finished_at ||= db_current_time
     when Cancelled
-      self.cancelled_at ||= Time.now
+      self.cancelled_at ||= db_current_time
     end
 
     # TODO: Remove the following case block when old "success" and
index da421ebb4cb92f13369a4517ef80583642d54844..4623393d0caf4371dfb366b6051df0f60fd72e82 100644 (file)
@@ -44,7 +44,7 @@ class KeepDisk < ArvadosModel
                               :last_read_at,
                               :last_write_at
                              ].collect(&:to_s).index k
-                           }.merge(last_ping_at: Time.now))
+                           }.merge(last_ping_at: db_current_time))
   end
 
   def service_host
index 39f789e69f9ebf89cf9ca00887f047a8f74b9578..b10a491163dc3c905c8ec52e120a6f263904457e 100644 (file)
@@ -48,7 +48,7 @@ class Log < ArvadosModel
     when "update"
       self.event_at = thing.modified_at
     when "destroy"
-      self.event_at = Time.now
+      self.event_at = db_current_time
     end
     self
   end
@@ -66,7 +66,7 @@ class Log < ArvadosModel
   alias_method :permission_to_delete, :permission_to_update
 
   def set_default_event_at
-    self.event_at ||= Time.now
+    self.event_at ||= db_current_time
   end
 
   def log_start_state
index c38f6817dbc768f4b380c51b999d75c6526dff61..bf27f6ff99104879da00019bd09bd7f975fbfd74 100644 (file)
@@ -61,12 +61,12 @@ class Node < ArvadosModel
 
   def status
     if !self.last_ping_at
-      if Time.now - self.created_at > 5.minutes
+      if db_current_time - self.created_at > 5.minutes
         'startup-fail'
       else
         'pending'
       end
-    elsif Time.now - self.last_ping_at > 1.hours
+    elsif db_current_time - self.last_ping_at > 1.hours
       'missing'
     else
       'running'
@@ -80,7 +80,9 @@ class Node < ArvadosModel
       logger.info "Ping: secret mismatch: received \"#{o[:ping_secret]}\" != \"#{self.info['ping_secret']}\""
       raise ArvadosModel::UnauthorizedError.new("Incorrect ping_secret")
     end
-    self.last_ping_at = Time.now
+
+    current_time = db_current_time
+    self.last_ping_at = current_time
 
     @bypass_arvados_authorization = true
 
@@ -88,7 +90,7 @@ class Node < ArvadosModel
     if self.ip_address.nil?
       logger.info "#{self.uuid} ip_address= #{o[:ip]}"
       self.ip_address = o[:ip]
-      self.first_ping_at = Time.now
+      self.first_ping_at = current_time
     end
 
     # Record instance ID if not already known
diff --git a/services/api/config/initializers/db_current_time.rb b/services/api/config/initializers/db_current_time.rb
new file mode 100644 (file)
index 0000000..1da7b86
--- /dev/null
@@ -0,0 +1 @@
+require 'db_current_time'
diff --git a/services/api/lib/db_current_time.rb b/services/api/lib/db_current_time.rb
new file mode 100644 (file)
index 0000000..5c97baa
--- /dev/null
@@ -0,0 +1,7 @@
+module DbCurrentTime
+  CURRENT_TIME_SQL = "SELECT clock_timestamp()"
+
+  def db_current_time
+    ActiveRecord::Base.connection.select_value(CURRENT_TIME_SQL).to_time
+  end
+end
index 09dece2660f38b1ecac09acaaa50543a4224698b..0418a94510d9e046564cd5b49db5f659bd194330 100644 (file)
@@ -167,4 +167,34 @@ class ArvadosModelTest < ActiveSupport::TestCase
     assert_includes(attr_a, "uuid")
     refute_includes(attr_a, "success")
   end
+
+  test 'create and retrieve using created_at time' do
+    set_user_from_auth :active
+    group = Group.create! name: 'test create and retrieve group'
+    assert group.valid?, "group is not valid"
+
+    results = Group.where(created_at: group.created_at)
+    assert_includes results.map(&:uuid), group.uuid,
+      "Expected new group uuid in results when searched with its created_at timestamp"
+  end
+
+  test 'create and update twice and expect different update times' do
+    set_user_from_auth :active
+    group = Group.create! name: 'test create and retrieve group'
+    assert group.valid?, "group is not valid"
+
+    # update 1
+    group.update_attributes!(name: "test create and update name 1")
+    results = Group.where(uuid: group.uuid)
+    assert_equal "test create and update name 1", results.first.name, "Expected name to be updated to 1"
+    updated_at_1 = results.first.updated_at.to_f
+
+    # update 2
+    group.update_attributes!(name: "test create and update name 2")
+    results = Group.where(uuid: group.uuid)
+    assert_equal "test create and update name 2", results.first.name, "Expected name to be updated to 2"
+    updated_at_2 = results.first.updated_at.to_f
+
+    assert_equal true, (updated_at_2 > updated_at_1), "Expected updated time 2 to be newer than 1"
+  end
 end