Commit graph

728 commits

Author SHA1 Message Date
George Krajcsovits 71fee62838
Comment fixes (#364)
* matcher.go: restore comment from upstream

There's no reason to remove this comment.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

* head_append.go: Cortex to Mimir

This repo is used in Mimir.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2022-11-23 12:26:20 +00:00
Jeanette Tan 0cd4063a02 Fix tests 2022-11-23 02:21:03 +08:00
Jeanette Tan 51cf003517 Merge remote-tracking branch 'upstream/main'
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2022-11-23 01:39:23 +08:00
Ganesh Vernekar 42633bd05c
Merge pull request #11485 from t00350320/prometheus-office
GetRefByhash() will query a label's ref with hash value rather than lset.Hash().
2022-11-16 15:09:49 +01:00
tanghengjian 982007ecab
GetRefByhash will query a label's ref with hash value rather than lset.Hash().
Signed-off-by: tanghengjian <1040104807@qq.com>
2022-11-16 14:13:59 +01:00
Oleg Zaytsev 8553a98267
Optimize postings offset table reading (#11535)
* Add BenchmarkOpenBlock

* Use specific types when reading offset table

Instead of reading a generic-ish []string, we can read a generic type
which would be specifically labels.Label.

This avoid allocating a slice that escapes to the heap, making it both
faster and more efficient in terms of memory management.

* Update error message for unexpected number of keys

* s/posting offset table/postings offset table/

* Remove useless lastKey assignment

* Use two []bytes vars, simplify

Applied PR feedback: removed generics, moved the label indices reading
to that specific test as we're not using it in production anyway, we're
just testing what we've just built.

Also using two []bytes variables for name and value that use the backing
buffer instead of using strings, this reduces allocations a lot as we
only copy them when we store them (this is optimized by the compiler).

* Fix the dumb bug

Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
Co-authored-by: Marco Pracucci <marco@pracucci.com>
2022-11-14 17:48:16 +01:00
Julien Pivotto 739494d81b
Fix alignment of atomic int64 (#11547)
* Fix atomix int64 placement
* Test main for 386

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
2022-11-09 11:18:49 +01:00
Ganesh Vernekar fa6e05903f
Merge pull request #11447 from prometheus/sparsehistogram
Add Support for Native Histograms

This PR merges all the coding work that has been done in sparsehistogram branch over the last 1 year into main branch.

Design doc on native histograms: https://docs.google.com/document/d/1cLNv3aufPZb3fNfaJgdaRBZsInZKKIHo9E6HinJVbpM/edit
Some sneak peak: https://www.youtube.com/watch?v=T2GvcYNth9U
2022-10-26 17:10:46 -04:00
Viacheslav Panasovets 3d2e18bad5
Fix time.Since() in defer. Wrap in anonymous function (#11489)
Function arguments in defer evaluated during definition of defer, not
during execution

Signed-off-by: Slavik Panasovets <slavik@google.com>

Signed-off-by: Slavik Panasovets <slavik@google.com>
2022-10-26 00:26:12 +02:00
Björn Rabenstein 503ffba49a
chunkenc: Slightly optimize xorWrite/xoRead (#11476)
With these changes, the "happy path" when the leading and trailing
number of bits don't need an update, fewer operations are needed.

The change is probably very marginal (no change in the benchmark added
here, but the benchmark also doesn't cover non-changing values), and
an argument could me made that avoiding pointers also has its
benefits.

However, I think that reducing the number of return values improves
readability. Which convinced me that I should at least propose this.

Signed-off-by: beorn7 <beorn@grafana.com>
2022-10-20 15:08:01 +05:30
Ganesh Vernekar 8ee4dfd40c
Fix the build after conflict resolution
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-12 17:59:42 +05:30
Ganesh Vernekar 648be89822
Merge remote-tracking branch 'upstream/main' into fix-conflict
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-12 14:20:02 +05:30
Ganesh Vernekar 8e29110949
Add/Improve unit tests for compaction with histogram (#11342)
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-12 13:31:12 +05:30
Ganesh Vernekar 507bfa46fd
Fix HistogramChunk's AtFloatHistogram()
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-12 10:38:13 +05:30
Signed-off-by: Jesus Vazquez 3362bf6d79
Fix merge conflicts
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Co-authored-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-11 22:53:37 +05:30
Jesus Vazquez 775d90d5f8
TSDB: Rename wal package to wlog (#11352)
The wlog.WL type can now be used to create a Write Ahead Log or a Write
Behind Log.

Before the prefix for wbl metrics was
'prometheus_tsdb_out_of_order_wal_' and has been replaced with
'prometheus_tsdb_out_of_order_wbl_'.

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>
Signed-off-by: Jesus Vazquez <jesusvazquez@users.noreply.github.com>
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
2022-10-10 20:38:46 +05:30
Ganesh Vernekar b522fb0b76
Merge remote-tracking branch 'upstream/main' into sync-prom 2022-10-10 18:07:39 +05:30
Ganesh Vernekar 8a4a659d73
Fix build and upgrade CI Go to 1.18.7
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-10 14:35:33 +05:30
Sonali Rajput 9165aedb49 Fixed broken link in tsdb README.md
Signed-off-by: Sonali Rajput <sonalirajput1088@gmail.com>
2022-10-07 16:20:20 +00:00
Jesus Vazquez e934d0f011 Merge 'main' into sparsehistogram
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>
2022-10-05 22:14:49 +02:00
Ganesh Vernekar 83d9ee3ab7
Merge remote-tracking branch 'upstream/main' into sync-prom
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-05 20:33:36 +05:30
Ganesh Vernekar d0a6488c74
Update metrics for histograms
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-10-03 13:48:59 +05:30
Bryan Boreham 9b31adc4e8
tsdb: fix up sort call with faster slices.Sort (#11380)
This call was added by PR #11075 merged before #11318 which changed all
similar calls to `sort.Sort` into a faster one.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-10-01 12:55:40 -04:00
Bryan Boreham 3330d85ba8
Replace sort.Strings and sort.Ints with faster slices.Sort (#11318)
Use new experimental package `golang.org/x/exp/slices`.

slices.Sort works on values that are directly comparable, like ints,
so avoids the overhad of an interface call to `.Less()`.

Left tests unchanged, because they don't need the speed and it may be
a cross-check that slices.Sort gives the same answer.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-30 20:03:56 +05:30
Bryan Boreham 7f2374b703
tsdb: faster postings sort with generic slices.Sort (#11054)
Use new experimental package `golang.org/x/exp/slices`.

Some of the speedup comes from comparing SeriesRef (which is an int64)
directly rather than through an interface `.Less()` call; some comes
from exp/slices using "pattern-defeating quicksort(pdqsort)".

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-30 20:01:32 +05:30
Ganesh Vernekar 83d738e263
Fix 'invalid magic number 0' bug (#11338)
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-28 21:43:58 +05:30
Ganesh Vernekar f34aeefe6e
Allow overlapping blocks by default (#11331)
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-28 19:17:54 +05:30
Robert Fratto 448cfda6c1
tsdb/agent: fix validation of default options (#9876)
* tsdb/agent: fix application of defaults

MaxTS was being incorrectly constrained to the truncation interval

* add more tests to check validation

* force MaxWALTime = MinWALTime if min > max

Signed-off-by: Robert Fratto <robertfratto@gmail.com>
2022-09-27 19:41:43 +05:30
Bryan Boreham d166da7b59
tsdb: stop saving a copy of last 4 samples in memSeries (#11296)
* TSDB chunks: remove race between writing and reading

Because the data is stored as a bit-stream, the last byte in the stream
could change if the stream is appended to after an Iterator is obtained.
Copy the last byte when the Iterator is created, so we don't have to
read it later.

Clarify in comments that concurrent Iterator and Appender are allowed,
but the chunk must not be modified while an Iterator is created.
(This was already the case, in order to copy the bstream slice header.)

* TSDB: stop saving last 4 samples in memSeries

This extra copy of the last 4 samples was introduced to avoid a race
condition between reading the last byte of the chunk and writing to it.

But now we have fixed that by having `bstreamReader` copy the last byte,
we don't need to copy the last 4 samples.

This change saves 56 bytes per series, which is very worthwhile when
you have millions or tens of millions of series.

* TSDB: tidy up stopIterator re-use

Previous changes have left this code duplicating some lines; pull
them out to a separate function and tidy up.

* TSDB head_test: stop checking when iterators are wrapped

The behaviour has changed so chunk iterators are only wrapped when
transaction isolation requires them to stop short of the end.
This makes tests fail which are checking the type.

Tests should check the observable behaviour, not the type.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Co-authored-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-27 19:32:05 +05:30
Bryan Boreham ff00dee262
tsdb: turn off transaction isolation for head compaction (#11317)
* tsdb: add a basic test for read/write isolation

* tsdb: store the min time with isolationAppender
So that we can see when appending has moved past a certain point in time.

* tsdb: allow RangeHead to have isolation disabled
This will be used when for head compaction.

* tsdb: do head compaction with isolation disabled
This saves a lot of work tracking appends done while compaction is ongoing.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-27 19:31:23 +05:30
Bryan Boreham d0607435a2
tsdb: remove chunkRange and oooCapMax from memSeries (#11288)
* tsdb: remove chunkRange from memSeries

chunkRange is the (oddly-named) configured duration for the head block.

We don't need a copy of this value per series. Pass it down where
required, and remove the copy.

The value in `Head` is only updated in `resetInMemoryState()`, which
also discards all `memSeries`.

* tsdb: remove oooCapMax from memSeries

oooCapMax is the configured maximum capacity for an out-of-order chunk.

Storing it per-series uses extra memory, and has surprising behaviour
if users change the value in config - series created before the change
will keep their old value.

Instead, pass it down where required, and remove the per-series value.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Co-authored-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-27 13:52:22 +05:30
Ganesh Vernekar 758e29258b
Add/Improve unit tests for compaction with histogram Part 2 (#11343)
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-23 14:01:10 +05:30
Jesus Vazquez c1b669bf9b
Add out-of-order sample support to the TSDB (#11075)
* Introduce out-of-order TSDB support

This implementation is based on this design doc:
https://docs.google.com/document/d/1Kppm7qL9C-BJB1j6yb6-9ObG3AbdZnFUBYPNNWwDBYM/edit?usp=sharing

This commit adds support to accept out-of-order ("OOO") sample into the TSDB
up to a configurable time allowance. If OOO is enabled, overlapping querying
are automatically enabled.

Most of the additions have been borrowed from
https://github.com/grafana/mimir-prometheus/
Here is the list ist of the original commits cherry picked
from mimir-prometheus into this branch:
- 4b2198d7ec
- 2836e5513f
- 00b379c3a5
- ff0dc75758
- a632c73352
- c6f3d4ab33
- 5e8406a1d4
- abde1e0ba1
- e70e769889
- df59320886

Co-authored-by: Jesus Vazquez <jesus.vazquez@grafana.com>
Co-authored-by: Ganesh Vernekar <ganeshvern@gmail.com>
Co-authored-by: Dieter Plaetinck <dieter@grafana.com>
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* gofumpt files

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Add license header to missing files

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Fix OOO tests due to existing chunk disk mapper implementation

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Fix truncate int overflow

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Add Sync method to the WAL and update tests

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* remove useless sync

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Update minOOOTime after truncating Head

* Update minOOOTime after truncating Head

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix lint

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Add a unit test

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Load OutOfOrderTimeWindow only once per appender

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Fix OOO Head LabelValues and PostingsForMatchers

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Fix replay of OOO mmap chunks

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Remove unnecessary err check

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Prevent panic with ApplyConfig

Signed-off-by: Ganesh Vernekar 15064823+codesome@users.noreply.github.com
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Run OOO compaction after restart if there is OOO data from WBL

Signed-off-by: Ganesh Vernekar 15064823+codesome@users.noreply.github.com
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Apply Bartek's suggestions

Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Refactor OOO compaction

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Address comments and TODOs

- Added a comment explaining why we need the allow overlapping
  compaction toggle
- Clarified TSDBConfig OutOfOrderTimeWindow doc
- Added an owner to all the TODOs in the code

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Run go format

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Fix remaining review comments

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix tests

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Change wbl reference when truncating ooo in TestHeadMinOOOTimeUpdate

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>

* Fix TestWBLAndMmapReplay test failure on windows

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Address most of the feedback

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Refactor the block meta for out of order

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix windows error

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Signed-off-by: Ganesh Vernekar 15064823+codesome@users.noreply.github.com
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
Co-authored-by: Ganesh Vernekar <ganeshvern@gmail.com>
Co-authored-by: Dieter Plaetinck <dieter@grafana.com>
Co-authored-by: Oleg Zaytsev <mail@olegzaytsev.com>
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
2022-09-20 22:35:50 +05:30
Bryan Boreham af6167df58
WAL loading: don't send empty buffers over chan (#11319)
If some shards did not get any samples mapped, the buffer will be empty
so sending it over the chan to `processWALSamples()` is a waste of time.

This is especially likely now we are checking `minValidTime` before
sending.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-20 19:43:30 +05:30
Ganesh Vernekar 2474c6fb2c
Error on amending histograms on append (#11308)
* Error on amending histograms on append

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Rename Matches to Equals

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-19 13:10:30 +05:30
Bryan Boreham d2701be53a
tsdb: remove chunk pool from memSeries (#11280)
The chunk pool belongs to the head not to the series. Pass it down where
required, and remove the copy of the pointer that `memSeries` was
holding.

`safeChunk` also needs to hold it, because in scenarios where it is used
we don't have a reference to the head. However it was already holding
`chunkDiskMapper` for the same reason, so no big change.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-15 13:22:09 +05:30
Björn Rabenstein 7ad36505d5
tsdb: Update comment about a possible space optimization (#11303)
See also #11195 for the detailed reasoning.

Signed-off-by: beorn7 <beorn@grafana.com>

Signed-off-by: beorn7 <beorn@grafana.com>
2022-09-15 13:11:57 +05:30
Bryan Boreham e49d596fb1
WAL loading: check sample time is valid earlier (#11307)
There's no point splitting a sample onto the right shard and checking
if the series needs to be re-mapped, if we're only going to discard it
once it arrives at `ProcessWALSamples()`. Simply discard it earlier.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-15 12:36:57 +05:30
Ganesh Vernekar d354f20c2a
Add a feature flag to control native histogram ingestion (#11253)
* Add runtime config to control native histogram ingestion

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Make the config into a CLI flag

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-14 17:38:34 +05:30
Ganesh Vernekar 83e11014dd
Remove unnecessary tsdb/tsdbutil/buffer.go (#11302)
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-13 19:36:32 +05:30
Ganesh Vernekar b2d01cbc57
Remove unnecessary code in encoding/decoding histograms (#11252)
* Remove unnecessary code in encoding/decoding histograms

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-09-13 19:30:54 +05:30
Bryan Boreham 136f8b0ebb
tsdb: comment reason for isolation tracking reads (#11301)
I find it useful to know why a restriction exists, to check whether that
reason still applies, or in which other places it might apply.

This is based on the note here:
https://github.com/prometheus/prometheus/pull/9270#pullrequestreview-743820956
on the PR where the original comment was added.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-13 11:11:03 +02:00
Bryan Boreham 176fa38e76 tsdb: in tests use labels.FromStrings
Replacing code which assumes the internal structure of `Labels`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-09 13:34:49 +02:00
Bryan Boreham 0437dd7cee tsdb/wal: in tests use labels.FromStrings
Replacing code which assumes the internal structure of `Labels`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2022-09-09 13:34:49 +02:00
Julien Pivotto ec6c1f17d1
Update dependencies (#11287)
Updating dependencies following CI changes and move to go 1.19

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
2022-09-09 13:28:55 +02:00
Julien Pivotto 96d5a32659
Update go to 1.19, set min version to 1.18 (#11279)
* Update go to 1.19, set min version to 1.18

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>

* Update golangci-lint

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
2022-09-07 11:30:48 +02:00
Ganesh Vernekar 8f755f8f35
Extend createHead in tests to support histograms
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-08-29 20:18:02 +05:30
Ganesh Vernekar f540c1dbd3
Add support for histograms in WAL checkpointing (#11210)
* Add support for histograms in WAL checkpointing

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix tests

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-08-29 17:38:36 +05:30
Ganesh Vernekar 6383994f3e
Improve WAL/mmap chunks test for histograms (#11208)
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-08-29 16:21:32 +05:30
Ganesh Vernekar d209a29a5b
Add unit test for histogram append and various querying scenarios (#11194)
* Add unit test for histogram append and various querying scenarios

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* make lint happy

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix tests

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix review comments

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
2022-08-29 15:35:03 +05:30