Skip to content

Commit

Permalink
Identify players by UUID and migrate existing databases to include th…
Browse files Browse the repository at this point in the history
…is. Fixes #538, fixes #568

This creates a new column in the lb-players table called UUID. If this is in the form of a UUID,
it's assumed to be a player. If not, it's assumed to be a server entity (zombie, sheep, or
WaterFlow, LavaFlow etc.). LogBlock will set the UUID of entities to "log_" plus their name
(i.e. log_zombie or log_sheep)

To assist this is a new class Actor, which wraps a name/UUID pair, with constructors that will
generate one from server entities, or SQL results. Every listener and every class in Consumer
needed to be updated to deal with this

As of yet, only the playername is displayed in results (although the queries do return UUID data).
Similarly, you can only query by name (the database stores the last name they have logged in as).
In addition, the WorldEdit hook has been disabled (is not compiled) since LB needs to be updated
to use their new API, and the LB code hook has to extract UUID information for insertion.

The UUID importer assumes any player with an onlinetime of 0 is a server-generated source, and set
the UUID as above (log_sheep etc.).  For everything else, it sends 100 names at a time to Mojang's
name->UUID service, and records them if available.  If no result is found, it records their UUID as
noimport_theirname.  As this is more likely than other updates to be interrupted mid-way, the
importer is tolerant of e.g. the column already being added, and will resume where it left off.
  • Loading branch information
frymaster committed Feb 14, 2015
1 parent d7f2ac5 commit e3dc430
Show file tree
Hide file tree
Showing 36 changed files with 596 additions and 277 deletions.
3 changes: 3 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@
<configuration>
<source>1.6</source>
<target>1.6</target>
<excludes>
<exclude>**/de/diddiz/worldedit/*.java</exclude>

This comment has been minimized.

Copy link
@md-5

md-5 Feb 14, 2015

Member

Why? Do we need to pull the old changes?

This comment has been minimized.

Copy link
@frymaster

frymaster Feb 14, 2015

Author Member

We now need to get the UUID as well as the name of the source, and since WE changed their API anyway, this was the easiest quick-fix. #542 needs updated and pulled. I was going to look at it tomorrow, unless someone else wants to?

</excludes>
</configuration>
</plugin>
</plugins>
Expand Down
85 changes: 85 additions & 0 deletions src/main/java/de/diddiz/LogBlock/Actor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package de.diddiz.LogBlock;

import static de.diddiz.util.BukkitUtils.entityName;
import org.bukkit.entity.Entity;
import org.bukkit.entity.EntityType;
import org.bukkit.entity.Player;

import java.sql.ResultSet;
import java.sql.SQLException;

public class Actor {

@Override
public int hashCode() {
int hash = 5;
hash = 79 * hash + (this.UUID != null ? this.UUID.hashCode() : 0);
return hash;
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
final Actor other = (Actor) obj;
if ((this.UUID == null) ? (other.UUID != null) : !this.UUID.equals(other.UUID)) {
return false;
}
return true;
}

final String name;
final String UUID;

public Actor(String name, String UUID) {
this.name = name;
this.UUID = UUID;

}

public Actor(String name) {
this(name, generateUUID(name));
}

public Actor(ResultSet rs) throws SQLException {
this(rs.getString("playername"),rs.getString("UUID"));
}

public String getName() {
return name;
}

public String getUUID() {
return UUID;
}

public static Actor actorFromEntity(Entity entity) {
if (entity instanceof Player) {
return new Actor(entityName(entity),entity.getUniqueId().toString());
} else {
return new Actor(entityName(entity));
}
}

public static Actor actorFromEntity(EntityType entity) {
return new Actor(entity.getName());
}

public static boolean isValidUUID(String uuid) {
try {
java.util.UUID.fromString(uuid);
return true;
} catch (IllegalArgumentException e) {
return false;
}
}

public static String generateUUID(String name) {
return "log_" + name;

}
}
11 changes: 7 additions & 4 deletions src/main/java/de/diddiz/LogBlock/BlockChange.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,31 @@ public class BlockChange implements LookupCacheElement
{
public final long id, date;
public final Location loc;
public final Actor actor;
public final String playerName;
public final int replaced, type;
public final byte data;
public final String signtext;
public final ChestAccess ca;

public BlockChange(long date, Location loc, String playerName, int replaced, int type, byte data, String signtext, ChestAccess ca) {
public BlockChange(long date, Location loc, Actor actor, int replaced, int type, byte data, String signtext, ChestAccess ca) {
id = 0;
this.date = date;
this.loc = loc;
this.playerName = playerName;
this.actor = actor;
this.replaced = replaced;
this.type = type;
this.data = data;
this.signtext = signtext;
this.ca = ca;
this.playerName = actor == null ? null : actor.getName();
}

public BlockChange(ResultSet rs, QueryParams p) throws SQLException {
id = p.needId ? rs.getInt("id") : 0;
date = p.needDate ? rs.getTimestamp("date").getTime() : 0;
loc = p.needCoords ? new Location(p.world, rs.getInt("x"), rs.getInt("y"), rs.getInt("z")) : null;
actor = p.needPlayer ? new Actor(rs) : null;
playerName = p.needPlayer ? rs.getString("playername") : null;
replaced = p.needType ? rs.getInt("replaced") : 0;
type = p.needType ? rs.getInt("type") : 0;
Expand All @@ -49,8 +52,8 @@ public String toString() {
final StringBuilder msg = new StringBuilder();
if (date > 0)
msg.append(Config.formatter.format(date)).append(" ");
if (playerName != null)
msg.append(playerName).append(" ");
if (actor != null)
msg.append(actor.getName()).append(" ");
if (signtext != null) {
final String action = type == 0 ? "destroyed " : "created ";
if (!signtext.contains("\0"))
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/de/diddiz/LogBlock/ChatMessage.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,20 @@ public class ChatMessage implements LookupCacheElement
{
final long id, date;
final String playerName, message;
final Actor player;

public ChatMessage(String playerName, String message) {
public ChatMessage(Actor player, String message) {
id = 0;
date = System.currentTimeMillis() / 1000;
this.playerName = playerName;
this.player = player;
this.message = message;
this.playerName = player == null ? null : player.getName();
}

public ChatMessage(ResultSet rs, QueryParams p) throws SQLException {
id = p.needId ? rs.getInt("id") : 0;
date = p.needDate ? rs.getTimestamp("date").getTime() : 0;
player = p.needPlayer ? new Actor(rs) : null;
playerName = p.needPlayer ? rs.getString("playername") : null;
message = p.needMessage ? rs.getString("message") : null;
}
Expand All @@ -30,6 +33,6 @@ public Location getLocation() {

@Override
public String getMessage() {
return (playerName != null ? "<" + playerName + "> " : "") + (message != null ? message : "");
return (player != null ? "<" + player.getName() + "> " : "") + (message != null ? message : "");
}
}
Loading

38 comments on commit e3dc430

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 14, 2015

Choose a reason for hiding this comment

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

  1. Is it possible to cause a log (e.g. chat) immediately after login, so the online time would be 0?
  2. Does this convert records of users who are already renamed?

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

  1. If someone had logged in and logged out straight away then yeah, they might not be converted, but I don't think much of value would be lost
  2. No - the Mojang API will return "null" for their name. In the database, we give them a dummy UUID of noimport_theirname. If the player connects to the server again, a new player record will be created for them. Their old actions will be searchable under their old name while their new actions will be logged under their current name. I've just written an FAQ entry for this

EDIT: After old names become claimable by other people, if someone has renamed away from Joe and someone else claims it, the importer will naturally import the new person's UUID. There's not a lot I can do about that. From what I can see, the LogBlock updater works - and fails - on exactly the same scenarios as Mojang's ops/ban/whitelist updater.

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 14, 2015

Choose a reason for hiding this comment

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

It's possible to get a UUID for an historical name, see here.

  • What happens when 'bob' renames to 'steve' and 'alex' renames to 'bob' and we run the conversion then? Wouldn't that mix everything up?

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

(Your link didn't link properly)

"What happens when 'bob' renames to 'steve' and 'alex' renames to 'bob' and we run the conversion then? Wouldn't that mix everything up?"

Yes. Names are reserved for their original owners for 37 days after they rename though, so as long as you run the migration before the 13th of March that can't happen.

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 14, 2015

Choose a reason for hiding this comment

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

I thought it was 30 days, however if bob renames to alex and we run the migration, wouldn't bob's records convert to 'noimport_bob'? If alex then renames to bob, we would suddenly see bob again and mess up again? I think this would only happen if alex hasn't logged in after the conversion, but that's not too unlikely to happen.

Sorry for broken link: http://wiki.vg/Mojang_API#Username_-.3E_UUID_at_time

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 14, 2015

Choose a reason for hiding this comment

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

Hint: If you don't find a UUID for a name (because it was renamed already), just make that API call with at=0 that will give you the UUID for the original user of that name. If the migration is ran before the 30 day limit, this shouldn't cause any trouble.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

if bob renames to alex and we run the migration

you'd have a record with name "bob" and a UUID of "noimport_bob". If alex had logged in before the migration (which would have already created a new record for him) his record would be name "alex" and UUID of "abcdef......" and that record would have all his recent actions in it. If you queried for "alex" you'd get his new stuff, if you queried for "bob" you'd get his old stuff

If alex then renames to bob

We'd still have a record with name "bob" and a UUID of "nomport_bob". The "alex" record would have its name changed to "bob" but the UUID would stay "abcdef......". So if you queried for name "alex" you now wouldn't get anything at all, if you queried for "bob" you'd get old AND new stuff

There currently isn't a way of querying, OR displaying, the UUID for the actions, but they ARE recorded. So in the future you'd be able to say "give me all the records for the player **currently named bob" (while at the moment it's "give me all the records for all the players who were called bob when we last saw them)

I thought it was 30 days

You can change your name once every 30 days. After than, you have 7 days to change back to your last name

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 14, 2015

Choose a reason for hiding this comment

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

Hmm, I'm not sure if you see the issue, but it might also be me not understanding it. let's imagine this case:

  1. We don't have LogBLock's uuid update (i.e. this commit) yet
  2. We have the player "NameA" (but not NameB) on our server, with some log records
  3. NameA renames to NameC
  4. We now have NameA and NameC log records
  5. We update LogBlock to this UUID update (before the 37 day limit)
  6. NameA is stored as noimport_NameA, NameC is stored as abcdefuuid
  7. 37 days later, NameB (a guy we don't know yet) renames to NameA
  8. At some later time, (the new) NameA joins our server. We don't know about NameB, we only know about NameA. The server finds noimport_NameA and overwrites that with the new NameA's uuid. Now when you search for NameA, you'll get the old and the new NameA records.

The ideal solution would be to have the old NameA and NameB "merged" under the same uuid, and the new NameA stored with the new guy's uuid.

Correct me if I'm wrong, that's what I figured from your description.
Thanks for clarifying the 30-37 day stuff.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

The server finds noimport_NameA and overwrites that with the new NameA's uuid

No, during migration is the only time it ever considers using name to track people. There would be an older record with NameA, uuid=noimport_NameA and new record created with NameA, uuid=cdef123...

From the end-user point of view, you wouldn't see the difference right now - it's show it all as being done by NameA - but if he changed his name again to NameD, then the noimport_NameA would show up as NameA, and the cdef123... stuff would show up as NameD

On my TODO list is outputting the UUID as part of query results, and querying by UUID

@mibby
Copy link

@mibby mibby commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

@frymaster Is this safe to use on a production server? LogBlock dev 225. http://ci.md-5.net/job/LogBlock/lastSuccessfulBuild/
Noticing this in the console log.
I operate a network so I have BungeeCord online status set to true while the server's online status set to false.

Names censored to protect user privacy.

    Line 286: [16:44:32] [Craft Scheduler Thread - 3/WARN]: [Consumer] Failed to add player playername1
    Line 312: [16:44:50] [Craft Scheduler Thread - 6/WARN]: [Consumer] Failed to add player playername2
    Line 350: [16:45:18] [Craft Scheduler Thread - 5/WARN]: [Consumer] Failed to add player playername3
    Line 372: [16:45:42] [Craft Scheduler Thread - 6/WARN]: [Consumer] Failed to add player playername4
    Line 378: [16:45:44] [Craft Scheduler Thread - 0/WARN]: [Consumer] Failed to add player playername5
    Line 393: [16:45:52] [Craft Scheduler Thread - 4/WARN]: [Consumer] Failed to add player playername6
    Line 402: [16:46:00] [Craft Scheduler Thread - 4/WARN]: [Consumer] Failed to add player playername7
    Line 403: [16:46:00] [Craft Scheduler Thread - 4/WARN]: [Consumer] Failed to add player playername8
    Line 404: [16:46:02] [Craft Scheduler Thread - 4/WARN]: [Consumer] Failed to add player playername9
    Line 433: [16:46:44] [Craft Scheduler Thread - 0/WARN]: [Consumer] Failed to add player playername10
    Line 444: [16:46:56] [Craft Scheduler Thread - 4/WARN]: [Consumer] Failed to add player playername11
    Line 477: [16:47:30] [Craft Scheduler Thread - 0/WARN]: [Consumer] Failed to add player playername12
    Line 509: [16:48:17] [Craft Scheduler Thread - 0/WARN]: [Consumer] Failed to add player playername13

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

@mibby do those usernames still exist?

@mibby
Copy link

@mibby mibby commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

@jomo I believe so. The error seems to be outputting when players log in on the server. Looking at the lb-players table, UUID column, their UUID seems to be listed as log_playername, as well as their name displayed in the playername column. Not the actual UUID string.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

their UUID seems to be listed as log_playername

That happens when the player had an onlinetime value of 0 ie has no time logged on the server. I'd be interested to know how that happened. The first step of the migration is assuming all entries with an onlinetime of 0 are serverside (Wither, Witch, WaterFlow etc.) and it doesn't even TRY to find UUIDs for them.

So I don't know why that's happening.

What should happen now is that it creates a new record for the player going forward, because it can't link their new actions to their old ones. This isn't happening because I overlooked the fact that the playername index is unique. A fix for that will be uploaded shortly

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

I'd be interested to know how that happened.

Maybe spawning on a pressure plate?

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

onlinetime measures the cumulative time they've spent on the server, in seconds. I don't see what pressure plates have to do with it

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

I thought that might trigger a log immediately when they log on, but that's bullshit 😆

The error seems to be outputting when players log in on the server

Maybe online time logging is/was disabled in the config?

@mibby
Copy link

@mibby mibby commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

I do have the following set in the config because it was the default config value and didn't seem to have any issues with rolling back players prior to the UUID update.

  logPlayerInfo: false

Even listed as false by default on the wiki. Didn't need it set to true as I have last IPs logged by Essentials and last login time logged by PermissionsEx.
https://github.com/LogBlock/LogBlock/wiki/Configuration

However now, it seems no new logs are even being recorded. Is there any way to migrate the data for players even though the online time value would be set to 0?

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

aaargh, I was not aware you could turn off logging that info. The problem is, I don't have any other way during migration to e.g. tell the difference between a player named "WaterFlow" and the server-side action, "WaterFlow"

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

@mibby , I'm about to push a new build. Before you run it (but after stopping the server), do two things:

  1. Go into phpmyadmin and drop the column UUID from the lb-players table. If you use a MySQL command line, the syntax is:
USE name-of-your-database;
ALTER TABLE `lb-players` DROP COLUMN UUID;
  1. Edit the logblock config and change the version field to '1.81'

The combination of both those things should force a re-migration.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

Also, that was two separate bugs (pre-existing tables weren't allowing duplicate names, and it wouldn't migrate people if you weren't logging playerinfo), so thanks for the report :)

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

...sigh, and I didn't commit a file first time around. You want build 227. This is a sign I need to go to bed :/

@mibby
Copy link

@mibby mibby commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

@frymaster Thanks. UUID column dropped and version field changed to 1.81. Updated to build 227, seems to have finally converted player tables to UUID.

[19:17:25] [Server thread/INFO]: Processed 4200 out of 4215
[19:17:25] [Server thread/INFO]: Processed 4215 out of 4215
[19:17:25] [Server thread/INFO]: Updating tables to 1.91 ...

However, some players couldn't be updated with their UUID? The UUID column has a mixture of UUID strings and log_playernames.

[19:17:21] [Server thread/WARN]: mibby not found - giving UUID of log_mibby
[19:17:21] [Server thread/WARN]: mibby2 not found - giving UUID of log_mibby2 
[19:17:21] [Server thread/WARN]: mibby3 not found - giving UUID of log_mibby3

I have 406 players in the log out of the supposed 4,215 conversions where that error occurred. These are valid premium player names, as the server is online only and some of these users were on prior to the restart.

If I had to throw out a random guess, my guess would be it reached the UUID lookup cap and couldn't fetch the UUID of those players?

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

I just checked, and mibby2 and mibby3 are not coming up as premium. Are those actual examples or placeholders? I'd need a real name to check what the Mojang webservice is saying

@mibby
Copy link

@mibby mibby commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

Sorry. Provided placeholder names to protect user privacy. Here's the bunch.

-snip-

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

I just checked the first 4, and they're all coming up as not current. What's probably happened is that those people have changed their name, meaning the importer won't get back a UUID when it asks about that name.

What this means is that, for those players, when they next log in, a new record will be created. All their previous actions will be logged under name e.g. Pfly with the UUID of log_Pfly, and new actions will be logged with e.g. newNamePfly and UUID abcde..... If that person should change their name again, it will not create yet another record, but will correctly update the existing one to say newernamePfly, UUID unchanged at abcde.....

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

...actually, I've just realised it won't update the name unless you have logplayerinfo on. That can be changed, but it can wait until morning.

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

there is a solution to this, as explained above

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

@mibby
Copy link

@mibby mibby commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

@jomo With said solution, how would you fix players who have already logged in before any solution is implemented, having new records already created? For example, say the name playerexample1 couldn't be found and was converted to log_playerexample1. However, he changed his name to userexample2 and has already logged in, creating a new record with a UUID for his current name. Both his former and current names are in the table. Would it then just be optimal to check if there are any duplicate / matching UUIDs in the database, then merge it into one listed record - being the current name and UUID?

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

Well that solution should be part of LogBlock before the migration is run. If you have already migrated you probably have to drop the UUID column and reset the version as explained by frymaster above to force re-migration.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

@jomo the problem with that is it fails if trying to migrate someone who was only added to the database after they changed their name

e.g. luminousFeline is a current user on my server. https://api.mojang.com/users/profiles/minecraft/luminousFeline?at=0 returns nothing, because actually his name at that time was rndmlyasmdud

If he'd only first joined our server after he'd changed his name, we'd not be able to find his UUID. I'm not willing to compromise UUID migration for people whose name matches for the sake of people who don't.

EDIT: I don't think it's possible to write a migration tool that does The Right Thing in all possible cases without human oversight. As such, I'm strongly inclined to leaving it as stands, because it's at least straightforward enough right now to explain to people

@mibby
Copy link

@mibby mibby commented on e3dc430 Feb 15, 2015

Choose a reason for hiding this comment

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

So I guess I'm out of luck for those 406 users who did change their name prior to the migration?

At least don't forget about fixing it so it updates the player name if logplayerinfo is off.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

Logging name bug filed under #577

I'm afraid so. I'd like to apologise, because if I'd written the UUID integration in a timely fashion, everyone would have migrated before name changing came in and we wouldn't be in this predicament, so sorry :(

I've written an FAQ that describes the situation? Does that tell you everything you need to know?

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 16, 2015

Choose a reason for hiding this comment

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

@frymaster my idea is to bulk query users as you already do and make the query via https://api.mojang.com/users/profiles/minecraft/username?at=0 only for the failedPlayers. You might want to use the user's last log's timestamp for at instead because using at=0 returns inconsistent results and Mojang likes to change their API's behavior sometimes.


I don't think it's possible to write a migration tool that does The Right Thing in all possible cases

It is, using the API with proper at parameter, even after March 13 and even if one user changes their name to a previously used name.
However, before March 13 it is best do migrate as explained above for the sake of speed.
After that it would be best to query all users with a proper at parameter, although that is going to require one API request per user and is very slow. We will most probably hit Mojang's ridiculous rate limit but that's inevitable.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

This wouldn't help the other guy in this thread (who isn't logging the last login times) and, in cases where a player has already logged in with a new name to the server, result in their UUID appearing twice in the database. I can see how to solve the second issue, but it becomes a rabbit-hole of more and more code covering smaller and smaller edge-cases and becoming more brittle, especially since it doesn't appear you can batch-query the API using the at parameter.

I'm not in charge of LB, and if the other devs frown at me meaningfully I'll write that code, but otherwise the only way it's being written is if someone else makes a PR. Otherwise I want to concentrate on adding in support for the new blocks, for worldedit, for item frames, for JSON-text output, among other things. I feel this is a more constructive use of my time, and more importantly, it'll be useful to me.

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 16, 2015

Choose a reason for hiding this comment

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

If you don't have login times, you can take the time of the actual logs from the player. Or use at=0 as a last fallback.

I would try a PR if you're happy to accept it.

@frymaster
Copy link
Member Author

Choose a reason for hiding this comment

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

I'd love a PR if:

  • It detects if this is resulting in a duplicate UUID, and changes all references to the newer (higher-numbered) playerid to the older one (then removes the newer row from the player table)
  • It's tuned to not result in HTTP 429 codes being sent from Mojang (related: is there any official docs on what rate is appropriate? The conventional wisdom, for example, is you can send 100 names at a time to he bulk API, but then look at LogBlock throwing IOException when trying to convert names to UUIDs #576 which was returning 429 on the very first request...)
  • If the prior constraint means migration takes ages, then it has to be activated only by an option in the config

@jomo
Copy link

@jomo jomo commented on e3dc430 Feb 16, 2015

Choose a reason for hiding this comment

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

According to wiki.vg

All public APIs are rate limited so you are expected to cache the results. This is currently set at 600 requests per 10 minutes but this may change.

As explained in #576 my approach would be to run until we hit the rate limit and then wait a few minutes

Please sign in to comment.