Go (basic) + Notion on Investments | Part 3: Optimising the system
19 min read
Part 3: Optimising the system
The job was deployed and running. 11 writes, 7.888 seconds.
I wanted to know where that went, so I broke it down. AMFI takes about a second. The other seven are eleven requests to Notion, one after another, each around ~600 milliseconds.
But my computer is not busy for those 600 milliseconds. It sends a few hundred bytes to Singapore and then sits there. No calculation, no work. Just waiting for a reply.
So the honest description of my program is that it spends ninety percent of its life waiting for other computers.
This fact turns out to be the whole truth. Some work is CPU bound, where the processor is genuinely busy and the only way to speed it up is more cores. Some work is I/O bound, where the processor is blocked on something outside itself. Mine is entirely the second kind.
For I/O bound work you do not need more power. You need the waiting to overlap. 1 requests that each wait ~600ms, waited for at the same time, cost 600ms rather than 6,600ms.
What Go does that Dart does not (I'm comparing with Dart as this is the language I write most of my code)
I already knew this shape from Flutter. Future.wait fires several requests and waits for them together, and it is why an app does not freeze while fetching.
Go looks similar and differs in one way that matters.
Dart runs a single isolate with an event loop. Work interleaves (meaning: to combine different elements so they alternate), but two pieces of code never execute at literally the same instant. Go schedules goroutines across every CPU core, so two really can run simultaneously. And they share memory by default.
Which opens a bug Dart cannot produce.
Our program keeps two counters, updated and failed. updated++ looks like one action. It is three:
Read the current value
Add one
Write it back
Two goroutines arrive at once, with the counter at 5:
A reads 5
B reads 5 <- before A has written anything
A computes 6, writes 6
B computes 6, writes 6
Two successful writes. Counter says 6. One increment has vanished.
In Dart this cannot happen. Between the read and the write, nothing else can run, because Dart only switches tasks at an await. There is no await inside updated++, so it is uninterruptible by construction. Go makes no such promise.
Deleting the problem instead of guarding it
There are two ways out. Put a lock around every access to the counters, or stop sharing the counters at all.
I picked the second, and the reasoning generalises. A lock is a promise you have to keep in every single place that touches the variable. Miss one and the race is back, with no compiler complaint. Add a second lock and you can deadlock by taking them in different orders.
The other option removes the shared variable entirely. Workers send their outcome down a channel. Only main ever touches the counters, and only after every worker has finished.
A race needs two goroutines touching the same memory. Remove the sharing and there is nothing left to guard.
This is what the Go proverb means in practice:
Do not communicate by sharing memory, share memory by communicating.
Send the outcome, not a description of it
My first sketch of what a worker sends back had a message field:
type work struct {
job string
msg string
}
That is wrong in a way worth naming. If the worker formats a message, the worker has already decided how to describe the outcome. main then receives a sentence and has to inspect the text to know what happened. You end up checking whether a string contains the word "failed", which is parsing your own prose.
type result struct {
isin string
pageID string
err error
}
err == nil means success. Non-nil means failure, and main can print it, count it, or later decide to retry a timeout but not a 400. All of that stays possible because the error arrived intact.
Pass values, not formatted strings. Formatting is a decision, and it belongs where the decision is made.
Same rule as keeping the NAV as a string in the parser, several sections ago. Deciding what something means is not the job of the code that fetches it.
Three clerks, one queue
The pool itself is small:
jobChannel := make(chan job)
resultChannel := make(chan result, len(jobs))
var waitGroup sync.WaitGroup
for i := 0; i < 3; i++ {
waitGroup.Add(1)
go func() {
defer waitGroup.Done()
for j := range jobChannel {
<-limiter.C
err := updateNAV(token, j.pageID, j.value, j.isoDate)
resultChannel <- result{isin: j.isin, pageID: j.pageID, err: err}
}
}()
}
Before I wrote it I assumed the hard part was dividing the work. 11 jobs, three workers, so four and four and three. I went looking for the Go way to slice a list into chunks.
There is no chunking. There is one channel, all three workers read from it, and whoever is free takes the next one.
It is a queue at a counter. Three clerks do not each get a third of the line handed to them at the door. There is one line, and each clerk serves the next person the moment they are free. Divide the line into fixed groups in advance and the clerk who draws three quick customers stands idle while another is still working through a slow one.
That is not a small difference here. My writes ranged from 366ms to 5.124 seconds inside a single run. Any split I picked in advance would have been the wrong one.
Do not divide work in advance. Let whoever is free take the next piece.
The loop body is where my assumption actually dies. for j := range jobChannel does not mean "loop over my share". It means "take the next one, and keep taking until there are none left". All three workers run that identical line against that identical channel, and Go guarantees each value goes to exactly one of them.
The door has to close
close(jobChannel) is one line, and it is doing something the code does not look like it needs.
A range over a channel does not end when the channel is empty. To a reader, empty and finished are the same thing: nothing here right now. It ends when the channel is closed.
for _, j := range jobs {
jobChannel <- j
}
close(jobChannel)
Leave that last line out and the workers finish the eleventh job, loop round, and block forever waiting for a twelfth that is never coming. waitGroup.Wait() then waits on three goroutines that will never return.
The clerks can see the line is empty. They cannot see that the shop has shut. Somebody has to lock the door.
A reader cannot tell slow apart from finished. Closing is how you say finished.
11 funds is not 11 writes
resultChannel is buffered and jobChannel is not. That asymmetry is the part I would have got wrong.
Start with the size, because it is not eleven.
resultChannel := make(chan result, len(jobs))
buildJobs walks a map[string][]string, ISIN to page IDs, and emits one job per page. The same fund held under two investors is two rows in Notion, two page IDs, two writes. That map was a decision from several sections back, made because a plain map[string]string silently kept only the last page and dropped the rest. It is still earning its place here. The buffer is sized off the work, not off the funds.
Then the reason it needs a buffer at all, which is the ordering back in main:
waitGroup.Wait()
close(resultChannel)
for r := range resultChannel {
main does not read a single result until every worker has exited. So the buffer is not smoothing out a burst. It has to hold every result at once, because nothing drains it until the last worker is gone. len(jobs) is not a generous guess. It is the smallest number that works, and it cannot overflow, because every job produces exactly one result.
Take the buffer away and this happens. main is blocked sending job four into an unbuffered jobChannel. All three workers are blocked sending their first result into an unbuffered resultChannel. Four goroutines, each waiting on one of the others.
I wanted to see that rather than argue about it, so I rebuilt the same pool with the buffer removed and ran it:
fatal error: all goroutines are asleep: deadlock!
goroutine 1 [chan send]:
main.main()
/.../main.go:40 +0x194
goroutine 35 [chan send]:
main.main.func1()
/.../main.go:34 +0x90
created by main.main in goroutine 1
/.../main.go:30 +0xfc
Line 40 is jobChannel <- j. Line 34 is resultChannel <- result{...}. The runtime named both sides of the standoff, and the exact lines, before the program had written anything.
There is a second way to get this wrong, and it is the same two lines in the other order. Close resultChannel before waitGroup.Wait() and a worker sends into a closed channel. That is panic: send on closed channel. Loud again.
The bugs that shout are the cheap ones
That is worth sitting with, because it is the opposite of everything else in this post.
A parser that skipped nine funds exited zero. A struct tag missing a quote compiled. A benchmark told me concurrency had made the program slower and I believed it. None of them said a word.
Get the concurrency wrong and Go halts the program, prints every goroutine, and points at the line. The part I was most nervous about writing is the part that refuses to fail quietly.
The bugs that crash are the cheap ones. Save your fear for the ones that exit zero.
There is one line in that worker I have not explained.
<-limiter.C
Three clerks, and before serving anyone, each one waits for a bell. I put it there because Notion allows about three requests a second, and I had three workers, and I thought those were the same sentence.
The knob I reached for was the wrong one
Notion allows roughly three requests a second. My instinct was to use three workers and call that the rate limit.
It is not. Those are two different things.
Worker count controls how many requests are in flight. Rate controls how often a new one starts.
Three workers, each request taking 600ms, gives about 5 requests a second. Over the limit. Now suppose Notion slows down and each request takes two seconds. The same three workers now produce 1.5 a second. Under the limit, and slower than I need to be.
So worker count gives a rate that drifts with the server's response time. When Notion is struggling, I would hit it hardest right as it was recovering. That is exactly backwards.
The fix is to control the rate directly, with one ticker shared by every worker:
limiter := time.NewTicker(334 * time.Millisecond)
defer limiter.Stop()
// inside each worker, before the request:
<-limiter.C
Rate limit the requests, not the workers. Worker count is a concurrency limit. A ticker is a rate limit. They are different knobs.
One ticker, shared. Give each worker its own and you have tripled the rate while believing you limited it.
It was slower
I ran it. 9.721 seconds.
The sequential version had been 7.888.
I had added goroutines, channels, a wait group and a rate limiter, and made the program twenty percent slower. That is not what the arithmetic said should happen.
So before changing anything, I measured properly. A timestamp around each request, and three runs of the same code:
Run 1: mostly 400-500ms per request, total 5.5s
Run 2: mostly 1.3-2.0s per request, total 8.2s
Run 3: mixed, total 7.0s
Same code. Same 11 requests. Fifty percent swing between runs.
Which meant my original conclusion was worthless. 7.888 against 9.721 is one sample of each, and both numbers sit comfortably inside the range I had just measured. I had not made the program slower. I had rolled a dice twice.
A benchmark with fifty percent run to run variance cannot measure a twenty percent improvement.
Nothing errored. Nothing warned. I made a confident wrong inference from real data, which is the same failure this whole project keeps producing in different costumes.
The honest numbers, once I took medians over several runs: about 5.3 seconds concurrent against 7.9 seconds sequential. A real improvement, just not one a single run could have told me.
Something odd about empty cells
While testing I cleared the nav and nav_date columns, and that run felt slow.
The obvious response is that it was noise. I had just proved I could not tell a real difference from a bad afternoon.
So I alternated. Clear the cells, run. Run again with them populated. Clear, run. Run again. Four runs, two conditions, interleaved so that drifting network conditions land on both.
The totals were unconvincing: 7.9 and 6.8 for empty, 5.6 and 5.0 for populated. A forty percent gap, which is inside the noise I had already measured.
The individual request times were not:
Empty: 1.465s 2.131s 1.62s 1.985s 1.252s 1.784s
Populated: 448ms 464ms 478ms 460ms 535ms 439ms
Three times the latency, at the median, consistently, alternating exactly with the condition.
The signal was in the distribution, not the total.
The totals hid it because one bad request drags a whole run. In one earlier run a single write took 5.124 seconds when the same fund had taken 366ms a minute before. That one number accounted for most of the run total.
When one sample dominates the total, the total is not measuring what you think.
Writing a value into an empty property costs Notion more than overwriting one that already has a value. Creating rather than updating. It changes nothing about how I run the job, since after the first day every cell is always populated. But I would not have found it by looking at run totals, and I would not have believed it without alternating.
The schedule that never fired
Two things left. Run the tests, and watch it fire on its own.
No test files
$ go test ./... -race
? go-tools [no test files]
My test was gone. I had written it, run it, watched it catch a real parser bug, and then never committed it. Somewhere in the refactoring it left the disk too.
Which means go test ./... had been reporting success for days. Not failing. Not warning. Just quietly finding nothing to run and exiting zero.
A test that is not in the repository is not a test.
I rewrote it and committed it this time.
Worth being precise about what -race proved once it passed, though. The race detector only flags races it actually observes, and my test never calls the worker pool. So it confirmed the parser works. It did not confirm the pool is safe.
The evidence for the pool is structural rather than empirical. The counters are only touched by main, after every worker has finished. There is no shared memory for two goroutines to fight over. That is why I chose channels over a lock, and it is a better guarantee than a test passing once.
20:30 came and went
I set the cron for 20:30 IST and nothing happened.
My first assumption was that I had got the UTC conversion wrong, since I had already made that mistake once. I checked. 00 15 UTC is 20:30 IST. Correct.
The Actions tab told the real story. Every NAV sync run said "Manually run by". And the workflow file itself had last been changed at 18:06.
I had committed the schedule after the time had already passed. The slot was gone before GitHub knew the schedule existed. Nothing was broken.
Two things I learned in the process of not needing them:
GitHub only runs scheduled workflows from the default branch. And a schedule set for a round number like :00 or :30 is competing with everyone else's, so those are the ones dropped first under load. An odd minute gets served more reliably.
A cron typo produces silence, not an error. Test it with a time five minutes away, not tomorrow.
Which is what I did. Set it ten minutes out, push, wait, and watch for a run that does not say "Manually run by".
The last silent failure
Looking back at all of it, the thing that keeps repeating is not really about Go.
A parser that skipped nine funds and printed six. A scanner that stopped early and reported success. A struct tag missing a quote that matched nothing and compiled. A hundred rows arriving where there were more. A benchmark that told me the opposite of the truth. A test suite with no tests in it. A cron that fired into a time that had already passed.
None of them crashed. Every one of them exited zero.
The hard part was not writing the code. It was learning that code which runs is not code which works.
Full optimised code:
package main
import (
"bufio" // Buffered I/O
"bytes"
"encoding/json"
"fmt" // Formatted I/O
"io"
"log" // Output with timestamps, logs to stderr
"net/http" // Network
"os"
"strconv"
"strings"
"sync"
"time"
)
const (
amfiNAVURL = "https://www.amfiindia.com/spages/NAVAll.txt"
notionVersion = "2025-09-03"
)
func main() {
start := time.Now()
token := os.Getenv("NOTION_TOKEN")
if token == "" {
log.Fatal("NOTION_TOKEN not set.")
}
dataSourceID := os.Getenv("NOTION_DATA_SOURCE_ID")
if dataSourceID == "" {
log.Fatal("NOTION_DATA_SOURCE_ID not set.")
}
rows, err := fetchNotionRows(token, dataSourceID)
if err != nil {
log.Fatal(err)
}
pageCount := 0
for _, ids := range rows {
pageCount += len(ids)
}
log.Printf("Found %d pages across %d ISINs in Notion", pageCount, len(rows))
navs, err := fetchNAVs(amfiNAVURL)
if err != nil {
log.Fatal(err)
}
log.Printf("Parsed %d ISINs from AMFI.", len(navs))
jobs, skipped := buildJobs(rows, navs)
updated, failed := runUpdates(token, jobs)
failed += skipped
log.Printf("Done in %s: %d updated, %d failed", time.Since(start).Round(time.Millisecond), updated, failed)
if failed > 0 {
os.Exit(1)
}
}
// NAV is one scheme's published net asset value.
//
// Value stays a string on purpose: some schemes publish "N.A." instead
// of a number, and a parser that dies on one bad row gives you nothing.
// Date is kept because the file is not uniformly current — dead schemes
// sit in it for years with their last published NAV.
type NAV struct {
Name string // field 3
Value string // field 4
Date string // field 5
}
// parseNAVs reads AMFI's semicolon-delimited report from any source.
//
// Taking an io.Reader rather than a URL means this same function works
// against a live response, a file saved to disk, or a string literal in
// a test. Fetching and parsing are separate jobs.
func parseNAVs(r io.Reader) (map[string]NAV, error) {
navs := make(map[string]NAV)
scanner := bufio.NewScanner(r)
for scanner.Scan() {
// We will be splitting the line into fields using the semicolon ";" as the delimiter.
fields := strings.Split(scanner.Text(), ";")
// Instrument's name as no semiolons, as these are human readable in response.
// We will add a guard here to ensure that we have at least 6 fields, as we are interested in fields 3, 4, and 5.
if len(fields) < 6 {
continue
}
// The header row passes the guard, but we don't want to include it in the map. We will skip it by checking if the first field is "Scheme Code".
// Scheme code is a number, so we can use strconv.Atoi to check if it is a number. If it is not a number, we will skip the row.
if _, err := strconv.Atoi(strings.TrimSpace(fields[0])); err != nil {
continue
}
// Creating the NAV struct with the required fields. We will trim the whitespace from the fields to ensure that we have clean data.
nav := NAV{
Name: strings.TrimSpace(fields[3]),
Value: strings.TrimSpace(fields[4]),
Date: strings.TrimSpace(fields[5]),
}
// There are two ISINs for each fund, one for growth and one for dividend.
// Indexing both ISINs in the map, so that we can look up the NAV by either ISIN.
for _, index := range []int{1, 2} {
isin := strings.TrimSpace(fields[index])
if isin == "" || isin == "-" {
continue
}
navs[isin] = nav
}
}
return navs, scanner.Err()
}
func fetchNAVs(url string) (map[string]NAV, error) {
res, err := httpClient.Get(url)
if err != nil {
return nil, err
}
defer res.Body.Close()
// A non-2xx is not a transport error. The request succeeded, the
// server just said no. Without this check an HTML error page parses
// cleanly to zero rows and the program reports success.
if res.StatusCode != http.StatusOK {
return nil, fmt.Errorf("AMFI returned %d", res.StatusCode)
}
return parseNAVs(res.Body)
}
// QueryResponse is Notion's reply to a data source query.
//
// Notion sends at most 100 rows at a time. HasMore and NextCursor are
// how it tells you there is more, and where to carry on from.
type QueryResponse struct {
Results []Page `json:"results"`
HasMore bool `json:"has_more"`
NextCursor string `json:"next_cursor"`
}
// Page is one row of the table.
//
// The real response carries around two hundred fields per row. This
// describes four of them. Go ignores everything it was not told about,
// so this code does not break when Notion adds something new.
//
// Note that rich_text is a list. A Notion text cell can hold several
// runs of differently styled text, so even a plain cell arrives as a
// list with one item in it.
type Page struct {
ID string `json:"id"`
Properties struct {
ISIN struct {
RichText []struct {
PlainText string `json:"plain_text"`
} `json:"rich_text"`
} `json:"isin"`
} `json:"properties"`
}
// notionRequest builds a request with the three headers every Notion
// call needs. One function owns this so the API version string exists
// in exactly one place.
func notionRequest(method, url, token string, body []byte) (*http.Request, error) {
var reader io.Reader
if body != nil {
reader = bytes.NewReader(body)
}
req, err := http.NewRequest(method, url, reader)
if err != nil {
return nil, err
}
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Notion-Version", notionVersion)
req.Header.Set("Content-Type", "application/json")
return req, nil
}
// fetchNotionRows returns every ISIN in the table, along with every
// page that holds it.
//
// The value is a slice, not a single string. I hold the same fund under
// two investors, so one ISIN can appear on several rows. A plain
// map[string]string would keep the last one and silently drop the rest.
func fetchNotionRows(token, dataSourceID string) (map[string][]string, error) {
rows := make(map[string][]string)
url := "https://api.notion.com/v1/data_sources/" + dataSourceID + "/query"
cursor := ""
for {
body := map[string]any{"page_size": 100}
// start_cursor is eft out entirely on the first request.
// Sending it as an empty string is an error.
if cursor != "" {
body["start_cursor"] = cursor
}
payload, err := json.Marshal(body)
if err != nil {
return nil, err
}
req, err := notionRequest("POST", url, token, payload)
if err != nil {
return nil, err
}
res, err := httpClient.Do(req)
if err != nil {
return nil, err
}
// Notion sends its refusal in the response body.
if res.StatusCode != http.StatusOK {
msg, _ := io.ReadAll(res.Body)
res.Body.Close()
return nil, fmt.Errorf("Notion query returned %d: %s", res.StatusCode, msg)
}
var page QueryResponse
err = json.NewDecoder(res.Body).Decode(&page)
// Not using defer here because, defer is tied to a function.
// Inside a loop they are not the same. Hence, not using defer here.
res.Body.Close()
if err != nil {
return nil, err
}
for _, p := range page.Results {
// A row with empty ISIN has an empty list
if len(p.Properties.ISIN.RichText) == 0 {
continue
}
// A cell holding only whitespace trims to "". Without this it
// becomes a map key and reappears later as a warning about a
// fund with no name.
isin := strings.TrimSpace(p.Properties.ISIN.RichText[0].PlainText)
if isin == "" || isin == "-" {
continue
}
rows[isin] = append(rows[isin], p.ID)
}
// Notion sends 100 rows at a time. Stop only when it says so.
if !page.HasMore {
break
}
cursor = page.NextCursor
}
return rows, nil
}
// One shared client for every request in the program.
//
// It holds the connection pool, so repeated calls to Notion reuse the
// same TCP and TLS connection instead of negotiating a new one each
// time. It also carries a deadline, which the default client does not
// have at all. Without one, a server that accepts the connection and
// then stalls hangs this program forever.
var httpClient = &http.Client{
Timeout: 30 * time.Second,
}
// updateNAVs writes 2 properties to 1 page.
//
// This is a PATCH, not a replacement.
func updateNAV(token, pageID string, nav float64, isoDate string) error {
// Each value is wrapped in a property type, because a number & date column
// accept different shapes
body := map[string]any{
"properties": map[string]any{
"nav": map[string]any{"number": nav},
"nav_date": map[string]any{
"date": map[string]any{"start": isoDate},
},
},
}
payload, err := json.Marshal(body)
if err != nil {
return err
}
req, err := notionRequest("PATCH", "https://api.notion.com/v1/pages/"+pageID, token, payload)
if err != nil {
return err
}
res, err := httpClient.Do(req)
if err != nil {
return err
}
defer res.Body.Close()
// Notion puts the reason in the body. A bare status code tells you
// nothing; the body names the property it objected to.
if res.StatusCode != http.StatusOK {
msg, _ := io.ReadAll(res.Body)
return fmt.Errorf("Notion returned %d: %s", res.StatusCode, msg)
}
return nil
}
type job struct {
isin string
pageID string
value float64
isoDate string
}
type result struct {
isin string
pageID string
err error
}
func buildJobs(rows map[string][]string, navs map[string]NAV) (jobs []job, skipped int) {
for isin, pageIDs := range rows {
nav, ok := navs[isin]
if !ok {
log.Printf("Warning: No NAV for %s", isin)
skipped += len(pageIDs)
continue
}
value, err := strconv.ParseFloat(nav.Value, 64)
if err != nil {
log.Printf("Skipping %s: Bad Value Value %q: %v", isin, nav.Value, err)
skipped += len(pageIDs)
continue
}
t, err := time.Parse("02-Jan-2006", nav.Date)
if err != nil {
log.Printf("Skipping %s: Bad Date %q: %v", isin, nav.Date, err)
skipped += len(pageIDs)
continue
}
isoDate := t.Format("2006-01-02")
for _, pageID := range pageIDs {
jobs = append(jobs, job{
isin: isin,
pageID: pageID,
value: value,
isoDate: isoDate,
})
}
}
return jobs, skipped
}
func runUpdates(token string, jobs []job) (updated, failed int) {
jobChannel := make(chan job)
resultChannel := make(chan result, len(jobs))
limiter := time.NewTicker(334 * time.Millisecond)
defer limiter.Stop()
var waitGroup sync.WaitGroup
for i := 0; i < 3; i++ {
waitGroup.Add(1)
go func() {
defer waitGroup.Done()
for j := range jobChannel {
<-limiter.C
t0 := time.Now()
err := updateNAV(token, j.pageID, j.value, j.isoDate)
log.Printf("%s took %s", j.isin, time.Since(t0).Round(time.Millisecond))
resultChannel <- result{isin: j.isin, pageID: j.pageID, err: err}
}
}()
}
for _, j := range jobs {
jobChannel <- j
}
close(jobChannel)
waitGroup.Wait()
close(resultChannel)
for r := range resultChannel {
if r.err != nil {
log.Printf("Failed %s (%s): %v", r.isin, r.pageID, r.err)
failed++
continue
}
updated++
}
return updated, failed
}