Skip to content

Commit

Permalink
Disregarding throughput updates that are the same as the current values
Browse files Browse the repository at this point in the history
Found that updateTable hangs if we pass the same throughput values as
the table currently has (see #79).  Refactored throughput parameter
validation to its own function, since we'll need to validate the
parameters for the other operations as well.

Expanded tests.
  • Loading branch information
ricardojmendez committed Dec 15, 2015
1 parent 50db456 commit 8574687
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 24 deletions.
73 changes: 51 additions & 22 deletions src/taoensso/faraday.clj
Original file line number Diff line number Diff line change
Expand Up @@ -574,10 +574,37 @@


(defn update-table-request "Implementation detail."
[table throughput]
(doto (UpdateTableRequest.)
(.setTableName (name table))
(.setProvisionedThroughput (provisioned-throughput throughput))))
[table {:keys [throughput]}]
(doto-cond
[_ (UpdateTableRequest.)]
:always (.setTableName (name table))
throughput (.setProvisionedThroughput (provisioned-throughput throughput))))


(defn- val-update-throughput
"Validates a throughput update against the current table description. It will
return the same dictionary if all is well, or dissociate the throughput update
if it's requesting that we set the same values as the table currently has
(see issue #79)"
[table-desc {:keys [throughput] :as params}]
params
(let [{read* :read write* :write} throughput
current-throughput (:throughput table-desc)
{:keys [read write num-decreases-today]} current-throughput
read* (or read* read)
write* (or write* write)
decreasing? (or (< read* read) (< write* write))]
(cond
;; Hard API limit
(and decreasing? (>= num-decreases-today 4))
(throw (Exception. (str "API Limit - Max 4 decreases per 24hr period")))
;; Don't send an update request for the same throughput values
(= throughput (select-keys current-throughput [:read :write]))
(dissoc params :throughput)
;; No change
:else params
))
)

(defn update-table
"Updates a table. Returns a promise to which the final resulting table
Expand All @@ -586,28 +613,30 @@
Possible values to update:
:throughput - {:read <units> :write <units>}\n\n
:throughput - {:read <units> :write <units>}
"
[client-opts table {:keys [throughput]} & [{:keys [span-reqs]
:or {span-reqs {:max 5}}}]]
(let [throughput* throughput
{read* :read write* :write} throughput*
{:keys [status throughput]} (describe-table client-opts table)
{:keys [read write num-decreases-today]} throughput
read* (or read* read)
write* (or write* write)
decreasing? (or (< read* read) (< write* write))]
(cond (not= status :active)
[client-opts table update-params & [{:keys [span-reqs]
:or {span-reqs {:max 5}}}]]
(let [table-desc (describe-table client-opts table)
status (:status table-desc)
val-params (->> update-params
(val-update-throughput table-desc))]
(cond (not= :active status)
(throw (Exception. (str "Invalid table status: " status)))
(and decreasing? (>= num-decreases-today 4))
(throw (Exception. (str "API Limit - Max 4 decreases per 24hr period")))
:else
(letfn [(async-update []
(as-map
(.updateTable (db-client client-opts)
(update-table-request table {:read read* :write write*})))
;; Returns _new_ descr when ready:
@(table-status-watch client-opts table :updating))]
;; If we are not receiving any actual update requests, or
;; they were all cleared out by validation, simply return
;; the same table description
(if (empty? val-params)
table-desc
(do
(.updateTable
(db-client client-opts)
(update-table-request table val-params))
;; Returns _new_ descr when ready:
@(table-status-watch client-opts table :updating))))
]

(let [p (promise)]
(future (deliver p (async-update)))
Expand Down
12 changes: 12 additions & 0 deletions test/taoensso/faraday/tests/main.clj
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,18 @@
(select-keys #{:read :write})))
)

;; Sending the same throughput as what the table has should have no effect
(do-with-temp-table
[created (far/create-table *client-opts* temp-table
[:artist :s]
{:throughput {:read 1 :write 1}
:block? true})
updated @(far/update-table *client-opts* temp-table {:throughput {:read 1 :write 1}})
]
; Both table descriptions are the same
(expect created updated)
)


;;; Test `list-tables` lazy sequence
;; Creates a _large_ number of tables so only run locally
Expand Down
4 changes: 2 additions & 2 deletions test/taoensso/faraday/tests/requests.clj
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@
"update-table"
(.getTableName
^UpdateTableRequest
(update-table-request :update-table {:read 1 :write 1})))
(update-table-request :update-table {:throughput {:read 1 :write 1}})))

(expect
(ProvisionedThroughput. 15 7)
(.getProvisionedThroughput
^UpdateTableRequest
(update-table-request :update-table {:read 15 :write 7})))
(update-table-request :update-table {:throughput {:read 15 :write 7}})))

(expect
"get-item"
Expand Down

0 comments on commit 8574687

Please sign in to comment.