Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency between curator and web #229

Closed
bernerdschaefer opened this Issue May 7, 2013 · 6 comments

Comments

Projects
None yet
3 participants
@bernerdschaefer
Copy link
Contributor

bernerdschaefer commented May 7, 2013

Since curation is a core prometheus feature, and reporting the current status on the HUD is not, this communication shouldn't happen through potentially blocking channel communication.

Proposal 1 (preferred)

Introduce a new struct which atomically reads/updates the curation state. It's simpler, and has the advantage that the curator and web are only synchronized when the status page is requested. Something like:

type CurationStatus struct {
  sync.Mutex
  state CurationState
}

func (s *CurationStatus) Replace(state CurationState) {
  s.Lock()
  defer s.Unlock()

  s.state = state
}

func (s *CurationStatus) State() CurationState {
  s.Lock()
  defer s.Unlock()

  return s.state
}

Proposal 2

Change the current status updates to nonblocking sends, e.g.:

// before: status <- CurationState{Active: false}
select {
case status <- CurationState{Active: false}:
}
@matttproud

This comment has been minimized.

Copy link
Member

matttproud commented May 7, 2013

Option no. 1 was what I was leaning toward in the beginning but was time-constrained against. Go ahead and implement that.

@matttproud

This comment has been minimized.

Copy link
Member

matttproud commented May 7, 2013

Also, I had not been aware of the non-blocking send idiom:

    var ch = someChannel
    select {
    case ch <-foo:
    // ch is able to receive.
    default:
    // ch is unavailable.
    }

Fascinating.

@bernerdschaefer

This comment has been minimized.

Copy link
Contributor Author

bernerdschaefer commented May 7, 2013

Yep. Works for reads as well: http://play.golang.org/p/ql0qSUVXeX

@matttproud

This comment has been minimized.

Copy link
Member

matttproud commented May 7, 2013

Great. Please keep this insights up. They are really appreciated. If you want to verbally work with me in compiling a list of smells, I will be happy to pay special attention to these in the design and code review process.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Dec 10, 2014

The old storage is gone. Closing this.

@juliusv juliusv closed this Dec 10, 2014

simonpasquier pushed a commit to simonpasquier/prometheus that referenced this issue Oct 12, 2017

Merge pull request prometheus#229 from prometheus/go-build-prereqs
Improve docs around random example build prerequisites.
@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.