Code COMMUNITYBOARD - RankingSystem (Top 15 PVP/PK)

[StinkyMadness]

I decided to share one Ranking System (Coded 1y ago) from my customized Community Board.

 

 

Code : https://pastebin.com/DQYi6xhy

[KingJK]

Beatiful thanks for share my friend.

[Freakazoid]

Great work. I like it. Plain and simple.

 

Just something to add: a personal position after the rankings so the player knows more or less where he/she stands at.

[Porretilla]

Thanks for sharing with us!

[Rootware]

[D3X]

I expect nothing less from you. 

[Zake]

Avoiding a ScheduledFuture task is a fancy way to "rest" your ThreadPool, although you have a vulnerable area (the one that you are doing db connection). This area can delay some times due to connection pool being busy or your query executing slow due to a lot of calculations. During that computation time (delay mentioned above) 2 or more threads (player clicks) can access this area at the same time aka multiple connections at the same time which will lead to duplicate data in your StringBuilder as StringBuilder is a not synchronized structure itself. You can either use StringBuffer, which is not prefered for the same reason CopyOnWriteArrayList is not prefered (slow writing operations) OR synchronize the vulnerable area. 

My suggestion:

 

private final Lock _connectionLock = new ReentrantLock()

private final void generateBuilder(...)
{
	if (!_connectionLock.tryLock())
		return;
	
	final StringBuilder sb = new StringBuilder();
	//probably more builders

	try
	{
		//connections and computations here

		_lastUpdate = System.currentTimeMillis(); OR _nextUpdate = ... (depends on your approach)
	}
	finally
	{
		//empty PVP/PK builder here
		//feed PVP/PK with the corresponding builder
		_connectionLock.unlock();
	}

 

During the vulnerable time this will return the old PVP/PK stored data when they are requested.

 

Also static fields are redundant in singleton classes so you can declare builders as

 

private final StringBuilder PVP = new StringBuilder();
private final StringBuilder PKS = new StringBuilder();

 

although CAPS is not a proper naming convention for those.

 

Edited August 10, 2021 by Zake

[StinkyMadness]

3 hours ago, Zake said:

This area can delay some times due to connection pool being busy or your query executing slow due to a lot of calculations. During that computation time (delay mentioned above) 2 or more threads (player clicks) can access this area at the same time aka multiple connections at the same time which will lead to duplicate data in your StringBuilder as StringBuilder is a not synchronized structure itself.

This code is used about one year in 3 server and never there existed any issue with duplicated StringBuilders. I will try to exploit and if I face that kind of issue I will update the share.

[Zake]

12 minutes ago, StinkyMadness said:

This code is used about one year in 3 server and never there existed any issue with duplicated StringBuilders. I will try to exploit and if I face that kind of issue I will update the share.

It's not quite easy to produce this with low population. You can always benchmark this. Fill your characters table with a lot of data, and create a bunch of db connection tasks to keep connection pool busy enough, so that you have time to execute 2 requests at the same time.

[StinkyMadness]

11 hours ago, Zake said:

It's not quite easy to produce this with low population. You can always benchmark this. Fill your characters table with a lot of data, and create a bunch of db connection tasks to keep connection pool busy enough, so that you have time to execute 2 requests at the same time.

I think the problem can be solved instant by just moving the "_nextUpdate = System.currentTimeMillis() + 60000L;" on top of the method so the Connection will not affect it as it will be instantly updated the _nextUpdate.

//Update (Can't edit first topic for some reason)
https://pastebin.com/RfmLz5ZW

Edited August 11, 2021 by StinkyMadness

[Zake]

7 hours ago, StinkyMadness said:

I think the problem can be solved instant by just moving the "_nextUpdate = System.currentTimeMillis() + 60000L;" on top of the method so the Connection will not affect it as it will be instantly updated the _nextUpdate.

//Update (Can't edit first topic for some reason)
https://pastebin.com/RfmLz5ZW

You may avoid duplicate connections this way, although if a new player asks for data while 1st one is still processing he will get temporarily an empty table. Even critical error maybe if table is half fed. You could remove final modifiers from your structures, then create temporary StringBuilders during connection time and change the address of the pvp/pk to these temps when they are fully fed.

 

Edit: gonna investigate about topic edit issue and let you know

Edited August 11, 2021 by Zake

[StinkyMadness]

12 minutes ago, Zake said:

You may avoid duplicate connections this way, although if a new player asks for data while 1st one is still processing he will get temporarily an empty table. Even critical error maybe if table is half fed. You could remove final modifiers from your structures, then create temporary StringBuilders during connection time and change the address of the pvp/pk to these temps when they are fully fed.

 

Edit: gonna investigate about topic edit issue and let you know

I guess the VPS/Dedicate has not that kind of issues on 2021.. they are not that slow.. also the empty StringBuilders not give critical error on the way the HTML is written.. maybe it will be empty but they can just reopen it

i want to update the link on main topic but i can't edit it for some reason.

[ZaNteR]

1 hour ago, StinkyMadness said:

I guess the VPS/Dedicate has not that kind of issues on 2021.. they are not that slow.. also the empty StringBuilders not give critical error on the way the HTML is written.. maybe it will be empty but they can just reopen it

i want to update the link on main topic but i can't edit it for some reason.

May i suggest you post it on https://gist.github.com ?
So you can keep updating it and the link wont change.

Edited August 11, 2021 by ZaNteR
grammar smh...

[StinkyMadness]

5 hours ago, ZaNteR said:

May i suggest you post it on https://gist.github.com ?
So you can keep updating it and the link wont change.

I suggest @Maxtor to review a bit the privileges  about people to be able to edit their topic
But thanks for the option

Edited August 11, 2021 by StinkyMadness

[melron]

-	public void showRakingList(Player player)
+	public synchronized void showRakingList(Player player)

 

@Zake ReentrantLock is a bad way to solve this 'problem' and does not need to be placed here for many reasons. Indeed, it might be show different html content rarely even while StringBuilder is way faster than StringBuffer cause is fat since is syncing all of its methods, but all you have to do, is an object synchronization (if stinky still want to keep it as it is) but not by using the reentrantLock here, in this case...

 

@StinkyMadness I would do the whole thing a bit different, like a manager which will storing the infos in custom classes and generate the content directly up on the player's request with a FloodProtector check. No sync at all

Edited August 11, 2021 by melron