Skip to content
This repository was archived by the owner on Oct 11, 2019. It is now read-only.

Conversation

@jkodumal
Copy link
Contributor

This is my proposed approach to bounding resource consumption in the go client. I also think this should be the reference model for all SDKs (save PHP and to a certain degree mobile) going forward. There are a number of ideas in here:

  • We're going to completely ditch the polling-for-a-single-feature model
  • There's a new updateProcessor abstraction, so we're more cleanly separating the process of updating the feature store from reading the feature store
  • We're replacing the polling-for-a-single-feature model with a poll-for-all-features model . This will be the backup option if streaming is disabled, and it is handled as a separate updateProcessor implementation
  • For simplicity right now, we ditch the attempt to fall back to polling if the stream is disconnected. It would be slick to dynamically switch back and forth between streaming and polling, but the state management is significantly more complex and the EventSource implementation doesn't support such transitions yet
  • Clients can choose whether to block on client initialization, with a configurable timeout
  • We no longer do lazy stream initialization. Instead, you have to decide whether or not your client should be offline in your config, and you can't dynamically change that.
  • The store is now accessed directly by the LDClient, and we no longer ask it whether it's initialized. This is potentially a big performance win because we no longer make a remote redis call to check initialization on every toggle call.

There's probably more here, so it's worth going over this in person as a group. But here is the first cut of it for early feedback.

}

// Stops the LaunchDarkly client from sending any additional events.
// Shuts down the LaunchDarkly client. After calling this, the LaunchDarkly client

Choose a reason for hiding this comment

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

comment on exported method LDClient.Close should be of the form "Close ..."

}

var ErrInitializationTimeout = errors.New("Timeout encountered waiting for LaunchDarkly client initialization")
var ErrClientNotInitialized = errors.New("Toggle called before LaunchDarkly client initialization completed")

Choose a reason for hiding this comment

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

exported var ErrClientNotInitialized should have comment or be unexported

@drichelson
Copy link
Contributor

This all seems reasonable. +1 for talking about this in person.

}

// Stops the LaunchDarkly client from sending any additional events.
// Shuts down the LaunchDarkly client. After calling this, the LaunchDarkly client

Choose a reason for hiding this comment

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

comment on exported method LDClient.Close should be of the form "Close ..."

@pkaeding
Copy link
Contributor

lgtm

jkodumal added a commit that referenced this pull request Feb 23, 2016
…tion

Bounded resource consumption refactoring
@jkodumal jkodumal merged commit 7f1362a into master Feb 23, 2016
@jkodumal jkodumal deleted the jko/bounded-resource-consumption branch February 23, 2016 06:56

store = config.FeatureStore

if !config.UseLdd && !config.Offline {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If LDD mode is set, then we will always timeout.

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.

5 participants