-
Notifications
You must be signed in to change notification settings - Fork 28
Support parsing HyperlocalExperiment #276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| package decoder | ||
|
|
||
| import ( | ||
| "context" | ||
| "database/sql" | ||
| "errors" | ||
| "golbat/db" | ||
| "golbat/pogo" | ||
|
|
||
| "github.com/jellydator/ttlcache/v3" | ||
| log "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| // Hyperlocal struct for hyperlocal experiment data | ||
| type Hyperlocal struct { | ||
| ExperimentId int32 `db:"experiment_id" json:"experiment_id"` | ||
| StartMs int64 `db:"start_ms" json:"start_ms"` | ||
| EndMs int64 `db:"end_ms" json:"end_ms"` | ||
| Lat float64 `db:"lat" json:"lat"` | ||
| Lon float64 `db:"lon" json:"lon"` | ||
| RadiusM float64 `db:"radius_m" json:"radius_m"` | ||
| ChallengeBonusKey string `db:"challenge_bonus_key" json:"challenge_bonus_key"` | ||
| UpdatedMs int64 `db:"updated_ms" json:"updated_ms"` | ||
| } | ||
|
|
||
| // HyperlocalKey represents the composite primary key for hyperlocal records | ||
| type HyperlocalKey struct { | ||
| ExperimentId int32 `json:"experiment_id"` | ||
| Lat float64 `json:"lat"` | ||
| Lon float64 `json:"lon"` | ||
| } | ||
|
|
||
| func (h *Hyperlocal) getKey() HyperlocalKey { | ||
| return HyperlocalKey{ | ||
| ExperimentId: h.ExperimentId, | ||
| Lat: h.Lat, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Urgh. I understand there may be no unique identifier (unusual for niantic though this is!) but I wonder if lat/lon as a float works reliably given the round-trip to database (I doubt it will be guaranteed byte-exact). We may be forced for the key to move to an integer representing a fixed precision - but I haven't seen the resolution of the data that comes from niantic for this - how many decimal places is it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be exact. From my data gathered three months ago, these fields are always truncated to 1e-6 (ignoring floating point rounding), e.g. Full row:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess if testing turns out to be problematic, we can store (int)(1e6*lat) instead. (It didn't seem problematic in the data I collected.) |
||
| Lon: h.Lon, | ||
| } | ||
| } | ||
|
|
||
| func (h *Hyperlocal) updateFromHyperlocalProto(data *pogo.HyperlocalExperimentClientProto, timestampMs int64) { | ||
| h.ExperimentId = data.ExperimentId | ||
| h.StartMs = data.StartMs | ||
| h.EndMs = data.EndMs | ||
| h.Lat = data.LatDegrees | ||
| h.Lon = data.LngDegrees | ||
| h.RadiusM = data.EventRadiusM | ||
| h.ChallengeBonusKey = data.ChallengeBonusKey | ||
| h.UpdatedMs = timestampMs | ||
| } | ||
|
|
||
| func getHyperlocalRecord(ctx context.Context, db db.DbDetails, key HyperlocalKey) (*Hyperlocal, error) { | ||
| // Check cache first using HyperlocalKey directly | ||
| if cachedItem := hyperlocalCache.Get(key); cachedItem != nil { | ||
| hyperlocal := cachedItem.Value() | ||
| return &hyperlocal, nil | ||
| } | ||
|
|
||
| hyperlocal := Hyperlocal{} | ||
| err := db.GeneralDb.GetContext(ctx, &hyperlocal, | ||
| `SELECT experiment_id, start_ms, end_ms, lat, lon, radius_m, challenge_bonus_key, updated_ms | ||
| FROM hyperlocal | ||
| WHERE experiment_id = ? AND lat = ? AND lon = ?`, key.ExperimentId, key.Lat, key.Lon) | ||
| statsCollector.IncDbQuery("select hyperlocal", err) | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return nil, nil | ||
| } | ||
|
|
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &hyperlocal, nil | ||
| } | ||
|
|
||
| func saveHyperlocalRecord(ctx context.Context, details db.DbDetails, hyperlocal *Hyperlocal) { | ||
| key := hyperlocal.getKey() | ||
| oldHyperlocal, _ := getHyperlocalRecord(ctx, details, key) | ||
|
|
||
| if oldHyperlocal != nil && !hasChangesHyperlocal(oldHyperlocal, hyperlocal) { | ||
| return | ||
| } | ||
|
|
||
| if oldHyperlocal == nil { | ||
| res, err := details.GeneralDb.NamedExecContext(ctx, ` | ||
| INSERT INTO hyperlocal ( | ||
| experiment_id, start_ms, end_ms, lat, lon, radius_m, challenge_bonus_key, updated_ms | ||
| ) VALUES ( | ||
| :experiment_id, :start_ms, :end_ms, :lat, :lon, :radius_m, :challenge_bonus_key, :updated_ms | ||
| ) | ||
| `, hyperlocal) | ||
| statsCollector.IncDbQuery("insert hyperlocal", err) | ||
| if err != nil { | ||
| log.Errorf("insert hyperlocal %+v: %s", key, err) | ||
| return | ||
| } | ||
| _ = res | ||
| } else { | ||
| res, err := details.GeneralDb.NamedExecContext(ctx, ` | ||
| UPDATE hyperlocal SET | ||
| start_ms = :start_ms, | ||
| end_ms = :end_ms, | ||
| radius_m = :radius_m, | ||
| challenge_bonus_key = :challenge_bonus_key, | ||
| updated_ms = :updated_ms | ||
| WHERE experiment_id = :experiment_id AND lat = :lat AND lon = :lon | ||
| `, hyperlocal) | ||
| statsCollector.IncDbQuery("update hyperlocal", err) | ||
| if err != nil { | ||
| log.Errorf("update hyperlocal %+v: %s", key, err) | ||
| return | ||
| } | ||
| _ = res | ||
| } | ||
| hyperlocalCache.Set(key, *hyperlocal, ttlcache.DefaultTTL) | ||
| } | ||
|
|
||
| func hasChangesHyperlocal(old *Hyperlocal, new *Hyperlocal) bool { | ||
| return old.StartMs != new.StartMs || | ||
| old.EndMs != new.EndMs || | ||
| old.RadiusM != new.RadiusM || | ||
| old.ChallengeBonusKey != new.ChallengeBonusKey || | ||
| old.UpdatedMs < new.UpdatedMs | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not considering lat/lon - there is a method for nearby precision comparison of floats
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They aren't considered because they are primary keys. :) |
||
| } | ||
|
|
||
| func ClearHyperlocalCache() { | ||
| hyperlocalCache.DeleteAll() | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,11 @@ type RawMapPokemonData struct { | |
| Timestamp int64 | ||
| } | ||
|
|
||
| type RawHyperlocalData struct { | ||
| Data *pogo.HyperlocalExperimentClientProto | ||
| Timestamp int64 | ||
| } | ||
|
|
||
| type webhooksSenderInterface interface { | ||
| AddMessage(whType webhooks.WebhookType, message any, areas []geo.AreaName) | ||
| } | ||
|
|
@@ -72,6 +77,7 @@ var playerCache *ttlcache.Cache[string, Player] | |
| var routeCache *ttlcache.Cache[string, Route] | ||
| var diskEncounterCache *ttlcache.Cache[uint64, *pogo.DiskEncounterOutProto] | ||
| var getMapFortsCache *ttlcache.Cache[string, *pogo.GetMapFortsOutProto_FortProto] | ||
| var hyperlocalCache *ttlcache.Cache[HyperlocalKey, Hyperlocal] | ||
|
|
||
| var gymStripedMutex = stripedmutex.New(128) | ||
| var pokestopStripedMutex = stripedmutex.New(128) | ||
|
|
@@ -82,6 +88,7 @@ var pokemonStripedMutex = intstripedmutex.New(1024) | |
| var weatherStripedMutex = intstripedmutex.New(128) | ||
| var s2cellStripedMutex = stripedmutex.New(1024) | ||
| var routeStripedMutex = stripedmutex.New(128) | ||
| var hyperlocalStripedMutex = intstripedmutex.New(128) | ||
|
|
||
| var s2CellLookup = sync.Map{} | ||
|
|
||
|
|
@@ -168,6 +175,11 @@ func initDataCache() { | |
| ttlcache.WithTTL[string, Route](60 * time.Minute), | ||
| ) | ||
| go routeCache.Start() | ||
|
|
||
| hyperlocalCache = ttlcache.New[HyperlocalKey, Hyperlocal]( | ||
| ttlcache.WithTTL[HyperlocalKey, Hyperlocal](60 * time.Minute), | ||
| ) | ||
| go hyperlocalCache.Start() | ||
| } | ||
|
|
||
| func InitialiseOhbem() { | ||
|
|
@@ -346,6 +358,39 @@ func UpdateStationBatch(ctx context.Context, db db.DbDetails, scanParameters Sca | |
| } | ||
| } | ||
|
|
||
| func UpdateHyperlocalBatch(ctx context.Context, db db.DbDetails, scanParameters ScanParameters, p []RawHyperlocalData) { | ||
| if len(p) <= 0 { | ||
| return | ||
| } | ||
|
|
||
| for _, raw := range p { | ||
| key := HyperlocalKey{ | ||
| ExperimentId: raw.Data.GetExperimentId(), | ||
| Lat: raw.Data.GetLatDegrees(), | ||
| Lon: raw.Data.GetLngDegrees(), | ||
| } | ||
|
|
||
| hyperlocalMutex, _ := hyperlocalStripedMutex.GetLock(uint64(key.ExperimentId) ^ math.Float64bits(key.Lat) ^ math.Float64bits(key.Lon)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if the hash would be better as a method on the key - ... I think the stripedmutex just calls the standard hashing interface from memory
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| hyperlocalMutex.Lock() | ||
|
|
||
| hyperlocal, err := getHyperlocalRecord(ctx, db, key) | ||
| if err != nil { | ||
| log.Errorf("getHyperlocalRecord: %s", err) | ||
| hyperlocalMutex.Unlock() | ||
| continue | ||
| } | ||
|
|
||
| if hyperlocal == nil { | ||
| hyperlocal = &Hyperlocal{} | ||
| } | ||
|
|
||
| hyperlocal.updateFromHyperlocalProto(raw.Data, raw.Timestamp) | ||
| saveHyperlocalRecord(ctx, db, hyperlocal) | ||
|
|
||
| hyperlocalMutex.Unlock() | ||
| } | ||
| } | ||
|
|
||
| func UpdatePokemonBatch(ctx context.Context, db db.DbDetails, scanParameters ScanParameters, wildPokemonList []RawWildPokemonData, nearbyPokemonList []RawNearbyPokemonData, mapPokemonList []RawMapPokemonData, weather []*pogo.ClientWeatherProto, username string) { | ||
| weatherLookup := make(map[int64]pogo.GameplayWeatherProto_WeatherCondition) | ||
| for _, weatherProto := range weather { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -438,7 +438,7 @@ func (stop *Pokestop) updatePokestopFromQuestProto(questProto *pogo.FortSearchOu | |
| } else { | ||
| infoData["pokemon_id"] = int(info.GetPokemonId()) | ||
| } | ||
| if info.ShinyProbability > 0.0 { | ||
| if info.ShinyProbability != 0.0 { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? equality is generally best avoided with floats (though 0 generally works as is a special float)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are unlikely to be a result of a floating point math calculation so I think precise comparison is fine. Also the old code is comparing with 0 as well instead of something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And yes 0.0 is a special float in proto, indicating default value. |
||
| infoData["shiny_probability"] = info.ShinyProbability | ||
| } | ||
| if display := info.PokemonDisplay; display != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| CREATE TABLE `hyperlocal` ( | ||
| `experiment_id` INT NOT NULL, | ||
| `start_ms` BIGINT NOT NULL, | ||
| `end_ms` BIGINT NOT NULL, | ||
| `lat` DOUBLE(18,14) NOT NULL, | ||
| `lon` DOUBLE(18,14) NOT NULL, | ||
| `radius_m` DOUBLE(18,14) NOT NULL, | ||
| `challenge_bonus_key` VARCHAR(255) NOT NULL, | ||
| `updated_ms` BIGINT NOT NULL, | ||
| PRIMARY KEY(`experiment_id`,`lat`,`lon`), | ||
| KEY `ix_end_ms` (`end_ms`,`lat`,`lon`) | ||
| ) ENGINE = InnoDB | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't normally force engine (some people use alternates), charset or collate (forcing and not using database default gives problems on a join)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I copied this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will PR the removal of these, this is a mistake |
||
| DEFAULT CHARSET = utf8mb4 | ||
| COLLATE = utf8mb4_general_ci; | ||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interested in others thoughts about moving updated to a ms field (where others are not). Consistency might be more important than other concerns (I'd be willing to consider a PR moving to a virtual updated and every other table being updated to ms as the per second resolution annoys me too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine to just do this instead to keep compatibility. I imagine this would break compatibility for a lot of other things if we introduce this to older tables though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would use a virtual column (as a calculated field) to maintain the old name. Anyway, for the time being we should change this to updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? This is a new table and RM PR is already implemented for this. Do you mean we should add a virtual
updatedcolumn for this instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also since the column explicitly gives the unit (which is actually a convenient consequence of following the same naming convention as proto), I think it's fine to just keep this as is.