fleetd_logs table

[dantecatalfamo]

#18234

• Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
  See Changes files for more information.
• Manual QA for all new/changed functionality
  • For Orbit and Fleet Desktop changes:
    • Manual QA must be performed in the three main OSs, macOS, Windows and Linux.
    • Auto-update manual QA, from released version of component to new version (see tools/tuf/test).

[roperzh]

very nice idea of using MultiLevelWriter, I took a quick look and left a couple of questions!

[roperzh]

do you know if the compiler is smart enough to not do a full copy for each row here? I wonder if it's worth it to store it as a string in the first place.

[dantecatalfamo]

This makes sense to me, I can store it as string. It will also stop it from re-interpreting the byte array of each record every time this table is queried

[roperzh]

can you sanity check my logic here?

1. Once we reached MaxEntries, won't this keep allocating and copying 10_000 elements to new arrays for the slices to point to?
2. Minor: can we initialize the slice capacity to avoid reallocation while the slice is filling in for the first time?

[dantecatalfamo]

Yeah so this is a good question. What should happen is that the underlying slice remains and we only only reference items 1-10_000 len=9_999, cap=. The array only gets reallocated once we exceed the cap. That being said, if we keep sliding the array over, at some point it will hit the cap and get reallocated. I'm actually not sure if it would re-allocate a larger slice or notice that even though we're at the end of the array, we only have len=10_000 and keep the allocation small

[dantecatalfamo]

Sorry I got the numbers wrong, we append to the array before shifting, it should be 10_001 and 10_000

[dantecatalfamo]

If you run this you'll see that that capacity keeps shrinking as we shift the array, and it only re-allocates the array once the len and cap match

package main

import (
	"fmt"
	"unsafe"
)

func main() {
	max := 25
	arr := []bool{}
	for range 1000 {
		arr = append(arr, true)
		if len(arr) > max {
			arr = arr[len(arr)-max:]
		}
		fmt.Printf("ptr=%p len=%d, cap=%d\n", unsafe.SliceData(arr), len(arr), cap(arr))
	}
}

[dantecatalfamo]

That being said, a ring buffer would probably be a better data structure for this since then we would never need to re-allocate anything

[roperzh]

yes, that's what I'm worried about (I don't know of a quick and easy way to sanity check). I'm concerned about the underlying array will keep on growing (using amortized constant time, so the copies are not that bad) but I think all the logs will be carried over.

check out this section: https://go.dev/blog/slices-intro#a-possible-gotcha

[roperzh]

I commented that ^ without seeing your code example, taking a look now

[dantecatalfamo]

If we add 1 million entries to a 10,000 big array, it only reallocs less than 300 times

package main

import (
	"fmt"
	"unsafe"
)

func main() {
	max := 10_000
	arr := []bool{}
	reallocs := 0
	for range 1_000_000 {
		origCap := cap(arr)
		arr = append(arr, true)
		if len(arr) > max {
			arr = arr[len(arr)-max:]
		}
		newCap := cap(arr)
		if origCap < newCap {
			println("reallocated")
			reallocs++
		}
		fmt.Printf("ptr=%p len=%d, cap=%d\n", unsafe.SliceData(arr), len(arr), cap(arr))
	}
	println("Reallocs:", reallocs)
}

Reallocs: 293

[dantecatalfamo]

Still more than we want though perhaps

[roperzh]

I'm okay with reallocations as they're amortized, I was worried about us keeping all the logs in memory. I think we proved that we're good! thank you!

just for kicks I also did this contrived example: https://go.dev/play/p/0qzpJ54-NR_j

[roperzh]

this is a tricky API, I learned this from Martin:

The problem with json.Decoder is that it consumes a stream of JSON values, so when it's done decoding one value, it is not guaranteed that it consumed the whole input (e.g. https://pkg.go.dev/encoding/json#example-Decoder). What we should do to enforce that is to check that it returns io.EOF (and eat that error as a success)

[dantecatalfamo]

Their console writer logger only processes one message at a time, so I suspect they only send one per write call. I'll add the code to process multiple entries though, it won't be hard

[roperzh]

I was thinking just checking for io.EOF? to prevent malformed input

[dantecatalfamo]

Malformed input should return a non-EOF error I think? io.EOF will only get returned if we read the input in a loop and there is no more data after the first object is parsed

[roperzh]

not always, take this for example:

{"foo": "bar"}{

here decoder will decode the first token, however the JSON is invalid. It's a footgun of the decoder, if you're decoding a single token you should keep consuming tokens until you reach io.EOF

in this case, since you're expecting a single token, we can fail if the next one is not io.EOF

[dantecatalfamo]

Understood, I have added support for io.EOF 🙂

[roperzh]

(you can see in the official docs I linked an elegant way to do it)