Skip to content
This repository was archived by the owner on Dec 7, 2022. It is now read-only.

compatible with slime metric reconfiguration #12

Merged
merged 3 commits into from
Nov 16, 2021

Conversation

cywang1905
Copy link
Contributor

@cywang1905 cywang1905 commented Nov 2, 2021

related to slime-io/slime#110

slime-metric-v2-architecture

@cywang1905 cywang1905 requested a review from YonkaFang November 2, 2021 01:56
go.mod Outdated
slime.io/slime/framework => github.com/slime-io/slime/framework v0.3.1
//slime.io/slime/framework => ../slime/framework
//slime.io/slime/framework => github.com/slime-io/slime/framework v0.3.1
slime.io/slime/framework => ../slime/framework
Copy link
Contributor

Choose a reason for hiding this comment

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

replace back to a specific version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will change after slime pr finished

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change to v0.3.3

@cywang1905 cywang1905 force-pushed the master branch 5 times, most recently from fd71e6e to a0b2470 Compare November 11, 2021 09:48
//r.interestMeta.Pop(req.NamespacedName.String())
delete(r.interestMeta, req.NamespacedName.String())
for k, v := range r.interestMeta {
r.InterestMetaCopy[k] = v
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed according to concept of copy on write

r.source.WatchAdd(types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace})
r.interestMeta[req.NamespacedName.String()] = true
for k, v := range r.interestMeta {
r.InterestMetaCopy[k] = v
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong use

Copy link
Contributor

Choose a reason for hiding this comment

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

m, mc map[string]string

func update() {
  // ...
  mut.Lock()
  delete(m, "key1")
  newMc := map[string]string{}
  for k, v := range m {
    newMc[key] = value
  }
  mc = newMc
  mut.Unlock()
}

func GetM() map[string]string {
  mut.RLock()
  return mc
  defer mut.RUnlock()
}

// call back function for ticker producer
func (r *ServicefenceReconciler) handleTickerEvent(event trigger.TickerEvent) metric.QueryMap {
r.reconcileLock.RLock()
defer r.reconcileLock.RUnlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

	r.reconcileLock.RLock()
        instMeta := r.interestMetaCopy
	r.reconcileLock.RUnlock()

      for ... {}

qm := make(map[string][]metric.Handler)
var hs []metric.Handler
for name, pHandler := range r.env.Config.Metric.Prometheus.Handlers {
query := strings.ReplaceAll(pHandler.Query, "$namespace", event.NN.Namespace)
Copy link
Contributor

Choose a reason for hiding this comment

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

we have two copies of this code block nearby

Copy link
Contributor Author

Choose a reason for hiding this comment

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

replace by function generateHandler

@cywang1905 cywang1905 changed the title WIP compatible with slime framework metric reconfiguration compatible with slime metric reconfiguration Nov 15, 2021
@cywang1905 cywang1905 self-assigned this Nov 15, 2021
@YonkaFang YonkaFang merged commit f778000 into slime-io:master Nov 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants