Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Lazy block head initialization #780

Merged
merged 3 commits into from
Jun 20, 2023
Merged

Lazy block head initialization #780

merged 3 commits into from
Jun 20, 2023

Conversation

kolesnikovae
Copy link
Contributor

@kolesnikovae kolesnikovae commented Jun 19, 2023

See grafana/pyroscope#2047 for context. This change makes PhlareDB to initialise the head lazily at the first ingestion request.

@kolesnikovae kolesnikovae changed the title Lazily block head initialization Lazy block head initialization Jun 19, 2023
Comment on lines -850 to -862
// Closes closes the head
func (h *Head) Close() error {
close(h.stopCh)

var merr multierror.MultiError
for _, t := range h.tables {
merr.Add(t.Close())
}

h.wg.Wait()
return merr.Err()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the method because it is impossible to use it correctly:

  1. Close is supposed to be called after Flush, and tables are closed at Flush, therefore the call always returns an error. It worked because we used to create an empty head right away after flush.
  2. There is no case when head is just closed without flushing data.

@kolesnikovae kolesnikovae marked this pull request as ready for review June 19, 2023 17:15
Copy link
Collaborator

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit 9538b6a into main Jun 20, 2023
17 checks passed
@cyriltovena cyriltovena deleted the fix/head-init-lazily branch June 20, 2023 07:14
simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
* Lazy block head initialization

* Add test for flush channel

* Fix lint issue
@kolesnikovae kolesnikovae self-assigned this Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants