ArangoDeploymentReplication resource
#143
Conversation
| @@ -0,0 +1,333 @@ | |||
| // | |||
| // Copyright 2017 ArangoDB GmbH, Cologne, Germany | |||
| // protected by copyright, patent and other intellectual and industrial | ||
| // property laws. Reverse engineering, disassembly or decompilation of the | ||
| // Programs, except to the extent required to obtain interoperability with | ||
| // other independently created software or as specified by law, is prohibited. |
There was a problem hiding this comment.
This looks different than copyright and Apache licenses in C++ sources. Is that proper?
| type ShardSyncInfo struct { | ||
| Database string `json:"database"` // Database containing the collection - shard | ||
| Collection string `json:"collection"` // Collection containing the shard | ||
| ShardIndex int `json:"shardIndex"` // Index of the shard (0..) |
| StatusMessage string `json:"status_message,omitempty"` // Human readable message about the status of this shard | ||
| Delay time.Duration `json:"delay,omitempty"` // Delay between other datacenter and us. | ||
| LastMessage time.Time `json:"last_message"` // Time of last message received by the task handling this shard | ||
| LastDataChange time.Time `json:"last_data_change"` // Time of last message that resulted in a data change, received by the task handling this shard |
There was a problem hiding this comment.
... and this is C style underscore. Is there a reason for different styles? Jan gets upset if I use C style.
| // and normal transaction synchronization is active. | ||
| SyncStatusRunning SyncStatus = "running" | ||
| // SyncStatusCancelling indicates that the synchronization process is being cancelled. | ||
| SyncStatusCancelling SyncStatus = "cancelling" |
There was a problem hiding this comment.
preferred spelling is one "l": canceling ... I was told by CTO at NuView to "fix it everywhere". Not sure how English spelling rules apply at ArangoDB.
| // until the synchronization has reached an `inactive` state. | ||
| // If this value is zero, the cancel function will only switch to the canceling state | ||
| // but not wait until the `inactive` state is reached. | ||
| WaitTimeout time.Duration `json:"wait_timeout,omitempty"` |
There was a problem hiding this comment.
again, these two timeouts use underscore ...
| } | ||
|
|
||
| type SetMessageTimeoutRequest struct { | ||
| MessageTimeout time.Duration `json:"messageTimeout"` |
There was a problem hiding this comment.
... while this timeout uses camel case.
| ForceTimeout time.Duration `json:"force_timeout,omitempty"` | ||
| } | ||
|
|
||
| type CancelSynchronizationResponse struct { |
There was a problem hiding this comment.
The declaration makes it seem that it is ok for this struct to return an empty Json object. Is that really acceptable?
| - arangorepl | ||
| singular: arangodeploymentreplication | ||
| scope: Namespaced | ||
| version: v1alpha |
There was a problem hiding this comment.
other things are v1beta1, this is v1alpha. does it matter?
There was a problem hiding this comment.
It's a choice. At some point we'll increase it.
| // Author Ewout Prangsma | ||
| // | ||
|
|
||
| package v1alpha |
There was a problem hiding this comment.
again this might be nothing, just seeing this v1alpha and remembering v1beta1 elsewhere. does it matter?
| func (s EndpointSpec) ResetImmutableFields(target *EndpointSpec, fieldPrefix string) []string { | ||
| var result []string | ||
| if s.GetDeploymentName() != target.GetDeploymentName() { | ||
| result = append(result, fieldPrefix+"deploymentName") |
There was a problem hiding this comment.
should "deploymentName" be pulled off the struct via reflection? This code appears to create two places where tag names are defined. If one place changes, will coder see second place too? My comment applies to "auth." and "xls." below.
There was a problem hiding this comment.
It could be, but that is (1) expensive and (2) really annoying code to look at.
I would prefer to make it a constant, but using constants in tags in not possible.
In the code here it is only for showing the user, so there is not critical semantics.
|
|
||
| // onUpdateArangoDeploymentReplication deployment replication update callback | ||
| func (o *Operator) onUpdateArangoDeploymentReplication(oldObj, newObj interface{}) { | ||
| o.Dependencies.LivenessProbe.Lock() |
There was a problem hiding this comment.
How is oldObj used? I only see it in the function declaration (or I am blind).
There was a problem hiding this comment.
It is not used. The reason it is still there, because this function is a callback with a predefined signature.
| // Called when the local storage was deleted by the user. | ||
| func (dr *DeploymentReplication) Delete() { | ||
| dr.deps.Log.Info().Msg("deployment replication is deleted by user") | ||
| if atomic.CompareAndSwapInt32(&dr.stopped, 0, 1) { |
There was a problem hiding this comment.
is dr.stopped automatically initialized to 0?
There was a problem hiding this comment.
Everything is go is initialized to its "zero" value. For ints that is 0.
| Int("capacity", ecap). | ||
| Msg("event queue buffer is almost full") | ||
| } | ||
| case <-dr.stopCh: |
There was a problem hiding this comment.
go ignorance here, if dr.stopCh has closed will ev still go to queue and all is ok?
There was a problem hiding this comment.
Yes. The cases of a select statement are tried in order.
There was a problem hiding this comment.
Of course it can happen that ev cannot be put into the queue (it is full) and stopCh is closed. In that case the stopCh case will win (as intended)
| } | ||
|
|
||
| // Update next interval (on errors) | ||
| if hasError { |
There was a problem hiding this comment.
there are several "err != nil" that generate log messages but do not set hasError. Should it be set in any of those cases?
This PR adds support for a new custom resource called
ArangoDeploymentReplication.This resource is used to configure dc2dc on kubernetes.