[BUG] Deadlock is possible in v1.6.0 instance manager

Describe the bug
This issue was originally reported by @eilyre in https://github.com/longhorn/longhorn/issues/7915. I am distilling it down to the root cause here.
It is possible for the v1.6.0 instance manager to experience a deadlock.
Every 10 seconds, checkMountPointStatusForEngine obtains the pm.lock. It may not give it up until it sends to p.UpdateCh.
https://github.com/longhorn/longhorn-instance-manager/blob/6372baca91084d36461ea860f8e7b45115a1a858/pkg/process/process_manager.go#L138-L151
p.UpdateCh is actually just pm.processUpdateCh.
https://github.com/longhorn/longhorn-instance-manager/blob/6372baca91084d36461ea860f8e7b45115a1a858/pkg/process/process_manager.go#L303
Each time it receives from pm.processUpdateCh, ProcessManager must obtain pm.lock.
https://github.com/longhorn/longhorn-instance-manager/blob/6372baca91084d36461ea860f8e7b45115a1a858/pkg/process/process_manager.go#L102-L104
We cannot receive from pm.processUpdateCh without obtaining pm.lock, but we cannot relinquish pm.lock until pm.processUpdateCh is clear.
@eilyre provided a goroutine dump that demonstrated the deadlock. There were thousands of goroutines waiting for the pm.Lock and the following two goroutines stuck as described above:
goroutine 185 [chan send, 5653 minutes]:
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).checkMountPointStatusForEngine(0xc0005cc700)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:149 +0x13b
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).startInstanceConditionCheck(0xc0005cc700)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:130 +0xe6
created by github.com/longhorn/longhorn-instance-manager/pkg/process.NewManager in goroutine 1
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:90 +0x365

goroutine 184 [sync.RWMutex.RLock, 5653 minutes]:
sync.runtime_SemacquireRWMutexR(0xc0008d29a0?, 0x58?, 0xc003518340?)
	/usr/local/go/src/runtime/sema.go:82 +0x25
sync.(*RWMutex).RLock(...)
	/usr/local/go/src/sync/rwmutex.go:71
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).startMonitoring(0xc0005cc700)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:104 +0xf3
created by github.com/longhorn/longhorn-instance-manager/pkg/process.NewManager in goroutine 1
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:89 +0x325

To Reproduce
I couldn't easily reproduce it without modifying instance-manager code to make it more likely.

Deploy Longhorn with the following. The custom instance-manager changes MountCheckInterval to 250 ms (from 10 s).

helm upgrade --namespace longhorn-system longhorn ./chart --set image.longhorn.instanceManager.repository=webericjames/longhorn-instance-manager,image.longhorn.instanceManager.tag=v5_20240213_short_mountCheckInterval_logging,preUpgradeChecker.upgradeVersionCheck=false,image.pullPolicy=Always

Modify `examples/statefulset.yaml with the following. This hammers a single node with parallel pod creations and deletions.

  replicas: 20 # default is 1
  podManagementPolicy: Parallel

      nodeSelector:
        kubernetes.io/hostname: eweber-v126-worker-9c1451b4-kgxdq # some node in the cluster

Apply examples/statefulset.yaml.
Run the following script.

i=1; while true; echo $i; i=$((i+1)); do kubectl delete -f examples/statefulset.yaml; sleep 15; kubectl apply -f examples/statefulset.yaml; sleep 15; done

Watch the Longhorn UI. Eventually, all volumes will be stuck detaching. It can take many iterations. In my most recent attempt, it took eight.

...
8
service "nginx" deleted
statefulset.apps "web" deleted
service/nginx created
statefulset.apps/web created

Set up a port forward to the instance manager's pprof endpoint.

eweber@laptop:~/longhorn> kl port-forward instance-manager-95b4f71659737f4e85d565d375f3fca1 6060:6060
Forwarding from <IP_ADDRESS>:6060 -> 6060
Forwarding from [::1]:6060 -> 6060

Confirm the symptoms at http://localhost:6060/debug/pprof/goroutine?debug=2.

goroutine 364724 [sync.Mutex.Lock, 9 minutes]:
sync.runtime_SemacquireMutex(0x7fc438589a68?, 0x30?, 0xc0000fc000?)
	/usr/local/go/src/runtime/sema.go:77 +0x25
sync.(*Mutex).lockSlow(0xc00003e270)
	/usr/local/go/src/sync/mutex.go:171 +0x15d
sync.(*Mutex).Lock(...)
	/usr/local/go/src/sync/mutex.go:90
sync.(*RWMutex).Lock(0xa707e2?)
	/usr/local/go/src/sync/rwmutex.go:147 +0x31
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).unregisterProcess.func1.1(0xc000528780, 0xc0008f40e0)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:334 +0x3e
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).unregisterProcess.func1()
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:342 +0x13d
created by github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).unregisterProcess in goroutine 364697
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:319 +0x147

goroutine 12 [chan send, 9 minutes]:
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).checkMountPointStatusForEngine(0xc000528780)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:149 +0x189
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).startInstanceConditionCheck(0xc000528780)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:130 +0x11c
created by github.com/longhorn/longhorn-instance-manager/pkg/process.NewManager in goroutine 1
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:90 +0x3a5

goroutine 11 [sync.RWMutex.RLock, 9 minutes]:
sync.runtime_SemacquireRWMutexR(0xc0000227e0?, 0x58?, 0xc00003cdb0?)
	/usr/local/go/src/runtime/sema.go:82 +0x25
sync.(*RWMutex).RLock(...)
	/usr/local/go/src/sync/rwmutex.go:71
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).startMonitoring(0xc000528780)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:104 +0x133
created by github.com/longhorn/longhorn-instance-manager/pkg/process.NewManager in goroutine 1
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:89 +0x365

Expected behavior
No deadlock occurs. This is the only bit of code that sends to the update channel under a lock. It's likely we can just modify that behavior.
Support bundle for troubleshooting
See https://github.com/longhorn/longhorn/issues/7915.
Environment

Longhorn version: v1.6.0
See https://github.com/longhorn/longhorn/issues/7915.
Additional context
The locking issue is a bit more complicated in the details than I initially supposed. In both of the code locations I identified above, we need to obtain a read lock with sync.RWMutex.RLock. Usually, two readers should both be able to obtain the lock simultaneously, leading to no deadlocking issue. However, in the goroutine dump, we also see the following goroutine:
goroutine 174521414 [sync.RWMutex.Lock, 5653 minutes]:
sync.runtime_SemacquireRWMutex(0x44674b?, 0x78?, 0x446780?)
	/usr/local/go/src/runtime/sema.go:87 +0x25
sync.(*RWMutex).Lock(0xc003f067c8?)
	/usr/local/go/src/sync/rwmutex.go:152 +0x6a
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).unregisterProcess(0xc0005cc700, 0xc0008d29a0)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:310 +0x34
github.com/longhorn/longhorn-instance-manager/pkg/process.(*Manager).ProcessDelete(0x1029e80?, {0xc00350c340?, 0x508886?}, 0xc00350c340)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/process/process_manager.go:284 +0xce
github.com/longhorn/longhorn-instance-manager/pkg/imrpc._ProcessManagerService_ProcessDelete_Handler({0x10db360?, 0xc0005cc700}, {0x1511968, 0xc00230dd10}, 0xc0043b7680, 0x0)
	/go/src/github.com/longhorn/longhorn-instance-manager/pkg/imrpc/imrpc.pb.go:1302 +0x169
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0005ec800, {0x1511968, 0xc00230dc80}, {0x15167a0, 0xc002784680}, 0xc0014c9320, 0xc0006ec600, 0x1cb6df8, 0x0)
	/go/src/github.com/longhorn/longhorn-instance-manager/vendor/google.golang.org/grpc/server.go:1372 +0xe03
google.golang.org/grpc.(*Server).handleStream(0xc0005ec800, {0x15167a0, 0xc002784680}, 0xc0014c9320)
	/go/src/github.com/longhorn/longhorn-instance-manager/vendor/google.golang.org/grpc/server.go:1783 +0xfec
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	/go/src/github.com/longhorn/longhorn-instance-manager/vendor/google.golang.org/grpc/server.go:1016 +0x59
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 174521376
	/go/src/github.com/longhorn/longhorn-instance-manager/vendor/google.golang.org/grpc/server.go:1027 +0x115

At the exact wrong moment, we tried to unregister a process. This requires a write lock. According to the RWMutex documentation, if a call to sync.RWMutex.Lock is blocked, no call to  sync.RWMutex.RLock can succeed. This probably contributes to the rarity of the observed deadlock. It can only occur if ProcessManager is actively registering/unregistering a process AND checkMountPointStatusForEngine is running at the exact same time.
// If a goroutine holds a RWMutex for reading and another goroutine might
// call Lock, no goroutine should expect to be able to acquire a read lock
// until the initial read lock is released. In particular, this prohibits
// recursive read locking. This is to ensure that the lock eventually becomes
// available; a blocked Lock call excludes new readers from acquiring the
// lock.

A side question, do we observe memory and CPU high consumption once there are many go routine stuck in this case? Maybe it could be triggered if we leave the deadlock for a long time and the go routine starts building up.

checkMountPointStatusForEngine was backported to 1.5 and 1.4 as well, so need to backport the fix accordingly. cc @ChanYiLin

Added this to https://github.com/longhorn/longhorn/wiki/Release-Known-Issues#v160.

Verified pass on longhorn master(longhorn-instance-manager d9b06ab9) with test steps

Can reproduce with the custom instance-manager image mentioned in test step, after change instance-manager image to master-head, the issue can not be reproduced.
Did not observe outstanding issue on master-head pipeline


backport: cmake prereq 2

Issue being fixed or feature implemented
depends on https://github.com/dashpay/dash/pull/6293
What was done?
Describe your changes in detail
How Has This Been Tested?
Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran
to see how your change affects other areas of the code, etc.
Breaking Changes
Please describe any breaking changes your code introduces
Checklist:
Go over all the following points, and put an x in all the boxes that apply.

[ ] I have performed a self-review of my own code
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have added or updated relevant unit/integration/functional/e2e tests
[ ] I have made corresponding changes to the documentation
[ ] I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

A few of the depends builds are running into this issue:
CMake Error at /usr/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:146 (message):
  Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the
  system variable OPENSSL_ROOT_DIR (missing: OPENSSL_CRYPTO_LIBRARY) (found
  version "1.1.1f")
Call Stack (most recent call first):
  /usr/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:393 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake-3.16/Modules/FindOpenSSL.cmake:447 (find_package_handle_standard_args)
  CMakeLists.txt:844 (find_package)
-- Configuring incomplete, errors occurred!

cannot reproduce locally, or really find anything about this in bitcoin. Any thoughts @UdjinM6

need https://github.com/bitcoin/bitcoin/pull/27496 to fix libevent build

@UdjinM6 Any ideas on following in windows depends build?
make[3]: windres: Command not found

@PastaPastaPasta try 0bbf6eb26

CI was happy but Guix build failed on my machine with "can't find cmake" or smth like that. Guix on develop looks good. Re-enabled Guix action for now and force-pushed with no changes to see if the issue can be reproduced here too.

try bitcoin#29725 it seems relevant

As part of this change we move cmake-minimal in Guix into the global package set.

Good catch @knst!
Thanks for pushing @UdjinM6, LGTM. Lets approve and merge?


test: useCurrencyConfig test

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.4%) to 44.562%

Totals

Change from base Build<PHONE_NUMBER>:
1.4%

Covered Lines:
465

Relevant Lines:
803

💛  - Coveralls


Game works in beta: Knights And Merchants

Accepting this, though we’re not tracking Wine usage so when it leaves Beta that comment will be removed.


9 make entire response object available in load callback

I added an instruction to retrieve the called services information.
newParams.response = that.reader.smd.services[that.name];

:shipit: Don't worry about the CI builds for now; I forgot we hadn't configured travis since this project uses maven. I'll file an issue to get the builds converted to grunt/gulp and we can tidy this up then.


DJ always appends _without_delay to certain method

DJ works fine on all methods except for this one. It does always append '_without_delay' to this methods and i don't know why.
This is the Foreman output:
03:06:41 worker.1 | [Worker(host:Drazens-MacBook-Air.local pid:98827)] VideoAsset#encode_without_delay completed after 0.0948
03:06:41 worker.1 | [Worker(host:Drazens-MacBook-Air.local pid:98827)] 1 jobs processed at 8.0676 j/s, 0 failed ...

This is the Model code:
class VideoAsset < ActiveRecord::Base

  include Rails.application.routes.url_helpers
  include VideoEncoder

  after_save :encode

  belongs_to :assetable, polymorphic: true
  has_many :zencoder_jobs, :uniq => true, :inverse_of => :video_asset, :dependent => :destroy

  # Validations
  validates :video, :poster, :presence => true

  # CarrierWave Mount Uploaders
  mount_uploader :poster, VideoAssetPosterUploader

  def encode
    # Encode only if the video file has changed (or is new)
    if self.video_changed?
      response = VideoEncoder.encode(self, notification_api_v1_zencoder_jobs_url)
      if response.success?
        job_id = response.body["id"]
        video_id = self.id
        ZencoderJob.create(:job_id => job_id, :video_asset_id => video_id, :encoded_state => "processing")
      end
    end
  end
  # Run this method always with DJ
  handle_asynchronously :encode
end

Is it maybe a problem to call the method in the after_save callback?
Using Ruby 2.0 and Rails 3.2.13

damn it. this thing should be highlighted in the readme.
If I understand it wee though :
In my app, i have let's say the following
after_save :do_delayed_stuff
def do_delayed_stuff
...
end
handle_asynchronously :do_delayed_stuff

Which always returns an error, undefined do_delayed_stuff_without_delay
If In understand what's explained here, I should have two methods :
def do_delayed_stuff
end

and
def do_delayed_stuff
end

with pretty much the same code in each (or a shortcut like you shown) if i wanted it all to work correctly
Am I right ?

No the way you modelled it in the first block is correct
after_save :do_delayed_stuff
def do_delayed_stuff
...
end
handle_asynchronously :do_delayed_stuff

He just explained what happens under the hood. You don#t have to do that manually. The error you get could be related to something different.

Damn so far DJ has been evil to me :/
I might be in a desperate need to understand what would be the entry point to understand I get this recurrent undefined {method}_without_delay error

Getting undefined method errors is usually a result of the worker not running the same code as the web process. There are many ways this can happen. Most involve a failed restart of the worker process after a code deploy. The easiest way to fix that is to stop all worker processes manually and start up a clean set of worker processes.

Feels like my mina deploy recipe is not doing the right thing
Thanks a lot for your reply @albus522 , feel more confident about it then.


LeetcodeTest always return Wrong Answer

the answer is :
class Solution {
public:
vector twoSum(vector& nums, int target) {
for(int i = 0; i < nums.size()-1; i++) {
for(int j = i+1; j< nums.size(); j++) {
if(nums[i] + nums[j] == target) {
return {i, j};
}
}
}
return {};
}
};

but submit can be accepted.

Hello, I would like to ask if you have encountered this problem when running tests

Excuse me , I would like to ask if you have encountered this problem when run

ning tests

Excuse me, I would like to ask if you have encountered this problem when running tests

I'm already logged in here and can see the topics I submitted on the leeetcode website, but I don't know why I can't submit them in neovim.

I used neovim, version 0.5

Excuse me, I would like to ask if you have encountered this problem when running tests

same problem


[C++][Parquet] onvertedType::NA is  written out in some cases

This makes it an invalid thrift enum.  ::NA is a placeholder enum internal to C++.  There will right out an enum value of 24 to thrift which can cause errors for other readers.  See: https://issues.apache.org/jira/browse/ARROW-10553
Reporter: Micah Kornfield / @emkornfield
Assignee: Antoine Pitrou](https://issues.apache.org/jira/secure/ViewProfile.jspa?name=apitrou) / @pitrou
PRs and other links:

GitHub Pull Request #9863

Note: This issue was originally created as PARQUET-1990. Please see the migration documentation for further details.

Antoine Pitrou / @pitrou:
I'll post a PR.

Micah Kornfield / @emkornfield:
Nice find on the reverted format change.   I added a DCHECK to ensure only value less then NA were written out and all tests still pass, I can open a PR with that change, or if you were thinking of ripping out NA entirely I can hold off.

Antoine Pitrou / @pitrou:
WTH, we have both ConvertedType::NONE and ConvertedType::NA

Micah Kornfield / @emkornfield:
Issue resolved by pull request 9863
https://github.com/apache/arrow/pull/9863

Antoine Pitrou / @pitrou:
WTH #2, https://github.com/apache/parquet-cpp/pull/360 added a converted type named "NA" in order to serialize Arrow null arrays.
https://github.com/apache/parquet-format/pull/45 added a converted type named "NA" in the Thrift definition.
https://github.com/apache/parquet-format/pull/51 then removed the converted type "NA" (before it was released) and replaced it with the logical type "NullType". But the Parquet C++ implementation wasn't updated to also remove "NA".

Antoine Pitrou / @pitrou:
Did we figure out how this value was written out?


Secure NS2Win connection to nightscout with API_SECRET or token

Dear @MrJordanRoth - thank you very much for the great NS2Win plugin to Rainmeter. It's working perfectly but there's one problem: The script only works on nightscout repositories which are "open" for reading to the whole internet. In detail this means, that it's mandatory to set AUTH_DEFAULT_ROLES="readable". Otherwise NS2Win currently cannot access the data via the pebble api. Obviously this is not a safe setting since everybody with the nightscout url can see the completely private data.
Suggestion: Could you implement secure access via authentication with the API_SECRET or ideally an access token?
Here's how this works: https://nightscout.github.io/nightscout/security/#create-a-token
I just tested this with my AUTH_DEFAULT_ROLES="denied" setting. This api call returns only the last BG entry, so preferably we'd use this for parsing: https://[your-nightscout-server-url]/api/v1/entries.json?count=1&token=[your-token-string]
This is where I'm lost currently -> how does the "RegExp" in line 40 of the NS2Win.ini script need to be configured to work with the above api output?
For comparison 1) "pebble api" output example:
{"status":[{"now":1664915998034}],"bgs":[{"sgv":"84","trend":4,"direction":"Flat","datetime":1664915722580,"bgdelta":-6,"battery":"86","iob":"0.59","bwp":"-0.31","bwpo":65,"cob":0}],"cals":[]}
For comparison 2) "api/v1/entries.json?count=1" output example:
[{"_id":"633c9910cdbc97b5c92e50e2","device":"xDrip-DexcomG5 G6 Native","date":1664915722580,"dateString":"2022-10-04T20:35:22.580Z","sgv":84,"delta":-6.005,"direction":"Flat","type":"sgv","filtered":78890,"unfiltered":75840,"rssi":100,"noise":1,"sysTime":"2022-10-04T20:35:22.580Z","utcOffset":120,"mills":1664915722580}]
Many thanks for looking into this and thereby improving the NS2Win user's nightscout database security. Happy to discuss, let me know if I can contribute or answer any questions you might have.

@emp-00 thanks for the suggestion, I haven't looked at this for awhile but I do see where this would be important. Let me look into this and update the code to include AUTH.

@emp-00, can you change the following in the .ini file for NS2Win:
Line 26: NightScoutURL=https://YOURNSDOMAIN/?token=YOURTOKEN
Line 39: URL=#NightScoutURL#/pebble
In Line 26 you will need to enter your website for NS and the token you configured as Readable. Line 39 should just be a simple copy/paste.
Let me know if that works for you please.

@emp-00, I think this can be accomplished with two simple changes in the .ini file.
Paste the following under Line 26:
NightScoutToken=/?token=ENTERNSTOKEN
Change Line 40:
URL=#NightScoutBaseURL#pebble#NightScoutToken#
Can you give it a test to see if this works?

@MrJordanRoth : THANK YOU - confirmed, this works like a charm! So the "?token=" command actually also works for the pebble api, that's super-great (I would have thought we'd need to switch to the /api/v1).
Thanks again, much appreciated!


Massive query spikes and significantly slower page load times in certain circumstances

Craft: 3.3.13
Relations: 1.2.1
We were recently having trouble optimizing one of our sites due to massive query spikes during page loads that caused very slow load times. We tracked it down to a number of Twig and PHP syntax choices that were avoidable but it kept bothering us that seemingly innocent bits of logic would cause these massive query spikes. I started running some tests in some very simple templates and I tracked it down to elements with field layouts that contained the Relations field. In my local environment, I deleted the field and the query spikes went away. But here are a few examples that affected performance:
144 queries
{% set oneCat = craft.categories.id(52032).one() %}
{% if oneCat %}

{{ oneCat.title|e }}

{% endif %}

2,000+ queries
{% set oneCat = craft.categories.id(52032).one() %}
{% if oneCat is not empty %}

{{ oneCat.title|e }}

{% endif %}

A PHP loop from a Yii/Craft module
<?php
foreach($category as $cat) {
    // This loop could be empty. Simply iterating over the array of elements executed thousands upon thousands of queries.
}

Sometimes we had template partials that could be used by a number of entry types and some of those entry types didn't have a particular relationship field so we would check for it using the Twig length filter. Field layouts lacking the relationship field would have a null value in that attribute. This check also caused massive query spikes in cases.
{# Massive query increase, slower page load. #}
{% set relatedEntries = entry.relatedEntriesField|length ? entry.relatedEntriesField : [] %}

{# No increase in queries. Page loads quickly. #}
{% set relatedEntries = entry.relatedEntriesField is not empty ? entry.relatedEntriesField : [] %}
{% set relatedEntries = entry.relatedEntriesField ? entry.relatedEntriesField : [] %}

My theory is that certain Twig operations cause Craft elements to execute their fields' normalizeValue() methods and I see this field type executes the queries when that method is called.

For me this slowed a simple action such as
$entry = new \craft\elements\Entry();
$entry->sectionId = 3;
$entry->typeId = 4;
Craft::$app->elements->saveElement($entry)

to take almost 2 minutes in local dev mode, writing tons of logs. It's a large DB with a few thousand entries, however the sections used in the relations field only have a few hundred.


WIP: Replace Guava with Caffeine.

Caffeine vs Guava
Caffeine
Benchmark                                         Mode  Cnt        Score       Error  Units
SingleNodeBenchmark.requestThroughput            thrpt<PHONE_NUMBER>.693 ± 29675.239  ops/s
SingleNodeBenchmark.avgRequestTime_batched        avgt  100        1.242 ±     0.007  us/op
SingleNodeBenchmark.avgRequestTime_singleThread   avgt  100        4.279 ±     0.060  us/op
Benchmark                                         Mode  Cnt        Score       Error  Units
SingleNodeBenchmark.requestThroughput            thrpt<PHONE_NUMBER>.027 ± 36085.998  ops/s
SingleNodeBenchmark.avgRequestTime_batched        avgt  100        1.269 ±     0.006  us/op
SingleNodeBenchmark.avgRequestTime_singleThread   avgt  100        4.142 ±     0.074  us/op
Guava
Benchmark                                         Mode  Cnt        Score       Error  Units
SingleNodeBenchmark.requestThroughput            thrpt<PHONE_NUMBER>.327 ± 35103.067  ops/s
SingleNodeBenchmark.avgRequestTime_batched        avgt  100        1.239 ±     0.010  us/op
SingleNodeBenchmark.avgRequestTime_singleThread   avgt  100        4.081 ±     0.078  us/op

@ben-manes excellent work with Caffeine, basically a drop in replacement for the Orbit project. Unfortunately I am seeing a bit of a performance drop when migrating from Guava 18.0 to Caffeine 2.2.3, any tips?

Note to self, check Failed tests:   testCacheDuration(cloud.orbit.actors.test.CacheResponseTest): expected:<2> but was:<3>

I suspect testCacheDuration is failing on CI because Guava amortizes maintenance work on the calling thread during read and write operations whereas Caffeine maintenance is asynchronous.

That's surprising, I'll need to run those benchmarks and investigate. The delegation to a background thread is to reduce response times, but in a benchmark is a penalty due to the extra context switches.
My initial suspicion is to compare their performance with weak keys, as it's not a case I've benchmarked. Guava's forked the hash table to optimize for reference collection, whereas I decorated instead and optimized for maximum size. That requires a wrapper lookup key, so especially on a lot of misses the extra GC would be a penalty.

Oh, and for tests the simplest is to set Caffeine.executor(Runnable::run) which is usually easy if you're using DI. If not then you're right using cleanUp is probably the simplest.
In the benchmark below the caches are built as Caffeine.newBuilder().weakKeys().build() and CacheBuilder.newBuilder().concurrencyLevel(CONCURRENCY_LEVEL).weakKeys().build(). In that scenario Guava wins on reads when not overwhelmed (contention on the per-segment counter). Since this is such a high penalty compared to a raw ConcurrentHashMap, it points to the lookup key wrapper as the culprit. The wrapper is created per lookup to compare by identity (identity hash and equals) which Guava can do directly in the hash table (via Equivalence<Object> keyEquivalence). I think the overhead would be smaller if I recycle the wrapper using a ThreadLocal instead, where the extra hashing will be a lower cost than the wrapper's GC penalty.
Can you quickly hack your changes to compare w/o weak keys enabled? That would be useful confirmation.
Benchmark                   (cacheType)   Mode  Cnt         Score          Error  Units
GetPutBenchmark.read_only            Caffeine  thrpt   10   78904420.319 ±  4338914.946  ops/s
GetPutBenchmark.read_only            Guava_4   thrpt   10   65837382.656 ±  4424105.297  ops/s
GetPutBenchmark.read_only            Guava_64  thrpt   10   85528167.301 ±  2885286.447  ops/s
GetPutBenchmark.read_only   ConcurrentHashMap  thrpt   10  259622836.318 ± 13136471.220  ops/s
GetPutBenchmark.readwrite            Caffeine  thrpt   10   72009974.220 ± 10445241.080  ops/s
GetPutBenchmark.readwrite             Guava_4  thrpt   10   46887322.041 ±  3901540.891  ops/s
GetPutBenchmark.readwrite            Guava_64  thrpt   10   71763490.732 ±  2384287.217  ops/s
GetPutBenchmark.readwrite   ConcurrentHashMap  thrpt   10  201025963.215 ±  7860052.150  ops/s
GetPutBenchmark.write_only           Caffeine  thrpt   10   29298544.080 ±   815351.349  ops/s
GetPutBenchmark.write_only            Guava_4  thrpt   10    4953373.921 ±   333589.144  ops/s
GetPutBenchmark.write_only           Guava_64  thrpt   10   19953776.079 ±   706751.768  ops/s
GetPutBenchmark.write_only  ConcurrentHashMap  thrpt   10   66842333.531 ±  5259936.811  ops/s

Due to escape analysis the wrapper isn't allocated on a read, so that wasn't the issue.
The cause is because ForkJoinTask.RunnableExecuteAction is allocated on every submission to wrap the runnable, which performs the cache's maintenance. For weakKeys this results in millions in the profiler, with only thousands for maximumSize. That's probably because the task execution is much faster, so the submission rate is higher. The solution is to allocate the a ForkJoinTask instead of the runnable and the pool reuses it. I remember seeing this before and I don't know how I forgot to fix it.
Benchmark                 (cacheType) Mode   Cnt          Score         Error  Units
GetPutBenchmark.read_only   Caffeine  thrpt   10  149676722.976 ± 6005882.472  ops/s
GetPutBenchmark.readwrite   Caffeine  thrpt   10  104603504.149 ± 6881197.985  ops/s
GetPutBenchmark.write_only  Caffeine  thrpt   10   37331074.722 ± 1432268.090  ops/s

In the case of a write the performance drop is due to the the wrapper allocation, as escape analysis fails. I think this is because the method is too long, so the JIT gives up on that optimization. Unfortunately simply moving the call to another method doesn't help reliably, as most of the time the JIT will decide to inline the call. This method is a challenge to break apart and I'm not sure if its worth the effort. The time when the JIT failed to inline provided a 10M boost, which is lower than I expected (should be closer to 60M ops/s).

Hi @johnou,
Can you retest using the SNAPSHOT? This contains the improvements described earlier. I haven't found a good way to speed up writes. That is still faster than Guava and should not be the source of a bottleneck.

Oh, I spoke too soon. Looks like I was able to raise the writeOnly throughput to 50M ops/s by pre-calculating the identity hash code. This probably won't affect most common usages, though, because it was only re-calculated on a retrial due to concurrent writes to the same entry. A retry is common in the benchmarks due to using a Zipf distribution to emulate real-world access patterns.

@ben-manes apologies for not getting back to you sooner, cat emergency. Impressive detailed analysis, I will try the latest SNAPSHOT now.

Those things take priority, there's no rush on my side.
The write improvement might have been a fluke, as I can't reproduce. Benchmarking on a laptop is error prone...

Ahh, I see now. It makes it easier for escape analysis to detect because the wrapper is a simple value object. If pulled into the retry loop the JIT might be smart and avoid the allocation. If outside then it sometimes is smart, sometimes allocates on the heap.

2.2.4-SNAPSHOT is almost on par, I will run another couple of benchmarks to verify.
Benchmark                                         Mode  Cnt        Score       Error  Units
SingleNodeBenchmark.requestThroughput            thrpt<PHONE_NUMBER>.063 ± 36487.435  ops/s
SingleNodeBenchmark.avgRequestTime_batched        avgt  100        1.239 ±     0.007  us/op
SingleNodeBenchmark.avgRequestTime_singleThread   avgt  100        4.228 ±     0.072  us/op

2.2.4-SNAPSHOT without weak values.
Benchmark                               Mode  Cnt        Score       Error  Units
SingleNodeBenchmark.requestThroughput  thrpt<PHONE_NUMBER>.122 ± 58227.558  ops/s

Hard to say. The cache might not be a bottleneck and the extra context switches are visible. You might try using Caffeine.executor(Runnable::run) to make Caffeine amortize on user threads. The main difference, though, is it delegates the removal notifications to the executor so might be less evenly spread out (Guava publishes to a queue, unlocks, and all requests help process it).
I tried running your benchmarks but it failed due to a connection issue (no server running?). The next step would be to profile Orbit and to benchmark with the other cache features that you're using (weakValues, expireAfterWrite) in case there's another oversight.

It may also be that cache.get(key, func) doesn't get escaped analysis so there is the GC issue on that code path.

Can you be more specific on what connection issue? SingleNodeBenchmark should run without any external services.

I might have misread it.
There are warnings in the warm-up,
14:40:27.560 [ForkJoinPool.commonPool-worker-1] WARN  org.jgroups.protocols.UDP - failed to join /<IP_ADDRESS>:7500 on utun0: java.net.SocketException: Can't assign requested address

and it fails after 20 iterations with,
Iteration  20: 14:41:10.209 [cloud.orbit.actors.test.SingleNodeBenchmark.requestThroughput-jmh-worker-3] WARN  cloud.orbit.actors.net.DefaultHandlerContext - Unprocessed write STOPPING
2182641.664 ops/s
<JMH had finished, but forked VM did not exit, are there stray running threads? Waiting 24 seconds more...>

It looks like there's some non-daemon executors?
Thread[pool-3-thread-5,5,main]
  at sun.misc.Unsafe.park(Native Method)
  at java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
  at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:2039)
  at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:1088)
  at java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:809)
  at java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1067)
  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1127)
  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
  at java.lang.Thread.run(Thread.java:745)

Yes there is, I should look into that. JMH kills the forks after 30 seconds so it slows down the entire benchmarking process. My colleague had the same issue as you, iirc he worked around it by disabling WIFI. I will change JGroups so you can run it without problems.
Below is benchmark with setting executor(Runnable::run)
Benchmark                               Mode  Cnt        Score       Error  Units
SingleNodeBenchmark.requestThroughput  thrpt<PHONE_NUMBER>.575 ± 43185.655  ops/s

Oh the another reason might be due to the ticker, which calls System.nanoTime(). On Mac/Windows that can be quite slow, especially with contention, due to polling hardware. On Linux it is very fast as a kernel thread updates a snapshot value, which is what all 3 platforms do with System.currentTimeMillis(). If Caffeine is faster then more contention on the time occurs and there is a net loss.
Can you try using a custom ticker with millis or just returning back a constant?
Sorry that this is a game of debugging and not a smooth migration.

Would you please try running SingleNodeBenchmark with -Djava.net.preferIPv4Stack=true? I am currently running benchmarks with .executor(Runnable::run).ticker(System::currentTimeMillis)

Will do.
Expiration is definitely causing a lot of overhead.
# System.nanoTime()
Benchmark  (cacheType) Mode  Cnt         Score         Error  Units
read_only   Caffeine  thrpt   10  34119040.917 ± 1237210.222  ops/s
read_only      Guava  thrpt   10  25222603.741 ± 1743789.896  ops/s
readwrite   Caffeine  thrpt   10  11702791.308 ± 2444290.601  ops/s
readwrite      Guava  thrpt   10  25538939.002 ±  715190.238  ops/s
write_only  Caffeine  thrpt   10   2381556.496 ±   71545.187  ops/s
write_only     Guava  thrpt   10   8410739.672 ±  206273.140  ops/s

# System.currentTimeMillis()
Benchmark(cacheType)   Mode  Cnt         Score         Error  Units
read_only   Caffeine  thrpt   10  68226450.796 ± 4746074.829  ops/s
read_only      Guava  thrpt   10  49128190.595 ± 1542803.952  ops/s
readwrite   Caffeine  thrpt   10  21119924.971 ± 6150580.330  ops/s
readwrite      Guava  thrpt   10  45130393.723 ± 1425360.779  ops/s
write_only  Caffeine  thrpt   10   2401491.244 ±  112033.233  ops/s
write_only     Guava  thrpt   10   7841423.896 ±  455622.536  ops/s

It looks like there's a regression in expiration and it will take me a while to figure out why it is so slow. Right now I see its much worse than expected, but I'm not sure yet why.

Sorry this is going to be a long explanation and you can ignore it. But if interested I think it is worth explaining why this slow down is observed.  I don't this affects your benchmarks much, but is visible in mine.

The issue only occurs for expireAfterWrite, as expireAfterAccess is only slowed down by acquiring the time. The problem is a case of queuing theory, thread scheduling, and a very high write rate.
In Guava a write blocks the segment and performs the all of the maintenance work for it thus far. This causes other writes (1 / n where by default n=4) to deschedule and wait their turn, as the operation is fairly slow given how broad it is. Reads progress and don't have to do the maintenance work because the writes are taking care of that. This penalizes writes over reads, but that's desirable because reads are the common case and latency sensitive. The parallelism of writes (N segments) helps to some degree.
Caffeine avoids segments as it doesn't use any stripe locks nor is ConcurrentHashMap segmented anymore. Just like how reads are recorded to a buffer and replayed under a lock for maintenance work, writes are buffered as well. The difference being that while the read buffer is lossy and scheduling can be delayed, writes must be kept and the maintenance work is scheduled immediately. This lets writers proceed and a single background thread takes care of the maintenance. In a real system this reduces user-facing latency and there's headroom to do this processing async.
In a benchmark there is no application business logic between writes and instead a writer gets the full cpu time slice to keep writing into the cache. With many writers into the queue and a single thread draining it, the overall performance begins to drop given the GC and other affects. I saw this in my maximumSize write stress tests for inserts, but it wasn't an issue because a Thread.yield() to emulate the real world resolved the issue.
In most updates it bypasses the write buffer and treats it as a read, e.g. an LRU reorder is the same in either case. It is only on a weight change or expireAfterWrite is that not true. The latter is because we order expiration on a FIFO queue, so like an LRU we can reorder in O(1) time and expire by peeking at the head. Because reads only reorder by access and we enqueue the node to avoid GC, the write buffer is used. This results in the same worse-case scenario under an artificial load that causes a degredation.
The solution is then to take the same fuzzy logic that memcached and others use - ignore it if within a small time window (e.g. 1 second). That classic trick is most often to avoid lock contention on reads for an LRU, but here we can use it to avoid overwhelming the write queue. If we observe the write rate is too high and that updating the entry's write time won't have a visible effect, we can ignore the work.
With that change the bottleneck becomes obtaining the system time, which is a slow operation.
# Using System.nanoTime() [high resolution]
Benchmark (cacheType) Mode  Cnt         Score         Error  Units
read_only  Caffeine  thrpt   10  33300860.072 ± 2059302.969  ops/s
read_only     Guava  thrpt   10  25696096.709 ± 1216610.169  ops/s
readwrite  Caffeine  thrpt   10  29352638.708 ± 1067015.454  ops/s
readwrite     Guava  thrpt   10  25962527.483 ±  530785.662  ops/s
write_only Caffeine  thrpt   10  27523761.263 ±  550693.590  ops/s
write_only    Guava  thrpt   10   8182072.940 ±  142045.763  ops/s

# Using System.currentTimeMillis() [medium resolution]
Benchmark  (cacheType) Mode  Cnt    Score          Error     Units
read_only  Caffeine  thrpt   10  65645152.423 ± 7583198.354  ops/s
read_only     Guava  thrpt   10  46095795.915 ± 5639592.232  ops/s
readwrite  Caffeine  thrpt   10  63341720.364 ± 2614699.225  ops/s
readwrite     Guava  thrpt   10  43824054.866 ± 1643528.467  ops/s
write_only Caffeine  thrpt   10  43482441.274 ± 2663772.527  ops/s
write_only    Guava  thrpt   10   9052547.148 ±  284201.709  ops/s

# Using volatile updated in background thread [low resolution]
Benchmark  (cacheType) Mode  Cnt      Score         Error      Units
read_only  Caffeine  thrpt   10  168529827.738 ± 10665487.560  ops/s
read_only     Guava  thrpt   10   76641814.064 ±  5860999.940  ops/s
readwrite  Caffeine  thrpt   10  134740502.806 ±  5478623.578  ops/s
readwrite     Guava  thrpt   10   69547375.148 ±  3179356.772  ops/s
write_only Caffeine  thrpt   10   68559250.821 ±  3713187.303  ops/s
write_only    Guava  thrpt   10   10120587.931 ±   200793.621  ops/s

You mean an impressive explanation, totally worth the read.

Think I found all the non-daemon threads which might cause the JVM to hang during benchmarks.

Thanks! I uploaded a new snapshot manually and pulled your branch, so I'm ready to give that another try.

There's still a ScheduledThreadPoolExecutor somewhere. Unfortunately the thread dumps don't backtrack to where it was created.

Oh I probably forgot to mvn install again.

Smashed it!
Benchmark                               Mode  Cnt        Score       Error  Units
SingleNodeBenchmark.requestThroughput  thrpt<PHONE_NUMBER>.362 ± 46281.930  ops/s

Most likely my previous commit wasn't enough, fingers crossed it's ok if you sync once more.

nice! How does it run without the same-thread executor?
Running fine now. I don't have a baseline to compare though, but will post the numbers when complete

The previous number was without the same-thread executor (assuming you are referring to the maintenance thread).

Yep, I was looking at ResponseCaching. What's the reason for leaving that with the executor defined?

cloud.orbit.actors.test.CacheResponseTest#testCacheDuration this test here, the CI tests were failing so I suspect it was related to read / write buffering?

oh most likely. If you can configure the executor for tests only (e.g. via dependency injection) then that avoids it for production. In this case it shouldn't make much of a difference, though.
I don't have a baseline so my numbers probably don't mean much.
requestThroughput            thrpt<PHONE_NUMBER>.618 ± 26832.215  ops/s
avgRequestTime_batched        avgt  100        1.048 ±     0.019  us/op
avgRequestTime_singleThread   avgt  100        2.194 ±     0.023  us/op

You could try it again with Caffeine 2.2.3?

Really appreciate the time and effort you have put in to help. If you are ever in Venice area drinks are on me.

Thanks for your help and patience. Same deal if you're ever in San Francisco.

Looks like a nice gain. Here's the v2.2.3 numbers.
requestThroughput            thrpt<PHONE_NUMBER>.598 ± 23785.454  ops/s
avgRequestTime_batched        avgt  100        1.262 ±     0.024  us/op
avgRequestTime_singleThread   avgt  100        2.404 ±     0.021  us/op

If you're happy then I'll cut a release.

Very happy, just wondering if the read / write buffer is going to cause any surprises in the other caches.

Guava used the read buffer, but its an unbounded CLQ with a logical bounding by a counter. That was the easiest way when I was originally figuring out the algorithms (in CLHM). Later I switched to ring buffers, but that never got into Guava. So for the read side this should be better.
CLHM only supports a maximum size, so the write buffer there is used less. Never heard any complaints from Cassandra, et al, so it seemed to be okay.
I think with these fixes there shouldn't be a problem anymore. I'd like to eventually move from an unbounded write buffer to a bounded one to reduce GC (linked nodes) and back-pressure if there is an unexpected runaway like here.

Makes sense, thanks again for explaning everything in such detail.

Released, but might take a while for Maven Central to sync.

Did you manage to get the perform maintenance task to throw an exception? If that's not caught it will stop running, right?

Pardon? It should keep running, its just not properly logged if an exception occurs. Usually an uncaught exception is handled by the executor by destroying the thread. The oversight of not logging would mask an issue if it occurs, but shouldn't break correctness. Or am I missing something?

I should rephrase, its not an infinite loop and maybe that's what you thought? The task would stop prematurely, but another one would be scheduled again later. An exception could come from a CacheWriter#delete() but that is a rarely used feature so its probable safe enough until the next release.

Mixed it up with a TimerTask, if an exception is thrown from its run method it stops running.

you might be right nonetheless. It might be intelligent and see that the task is complete and can no-op. That might be why I didn't do this optimization before. I better check. Might require another release

Okay, thanks! I need to make another release, unfortunately.
The executor will ignore the task as complete, so the maintenance work is busted. I'm surprised my tests get by with a single clean-up.
Thankfully there is a ForkJoinTask#reinitialize() to reset the internal state prior to submitting. So its an easy fix and won't require regressing to creating garbage.

Not just a timer http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/ScheduledExecutorService.html
"If any execution of the task encounters an exception, subsequent executions are suppressed."

Yes, for any scheduled task that's true. This is scheduled by user operations instead of a timer, so I assumed re-submitting a task (e.g. Runnable) was okay to recycle. An exception wouldn't halt a re-submission. Here, though, the pool is intelligent because tasks are forked and threads may help complete if there is a backlog.

Committed. Can you take a look and let me know if you see any other oversights? I'll release again if we're good.

After doing a bit of reading I wonder if we might need more checks. Java docs state, you have to be sure a task is done and
that no other thread/task is depending on its completion before you can reinitialize/refork.

The submission to the executor is guarded by a state machine (see scheduleDrainBuffers). In addition a tryLock is used to ensure exclusive access as others (primarily clear()) will lock to traverse the full data structure. The net result is that the task is submitted only once per completion event, so this should be safe. It isn't forked and there is no one awaiting its completion.
I assumed that JavaDoc was intended to warn that any downstream waiters queue'd actions would be broken. I don't think that applies to here.

LGTM! Nice catch.

Thank you! You caught it :-)

I'm not sure if you can mix ForkJoinTask and Runnable like that to get the desired performance boost?

At least with my tests when using ForkJoinPool it doesn't even compile (ambiguous method call) execute(ForkJoinTask<?> task) / execute(Runnable task)

It compiles because I'm using an Executor interface, not ForkJoinPool.commonPool() directly. Internally it checks to avoid re-wrapping.
public void execute(Runnable task) {
    if (task == null)
        throw new NullPointerException();
    ForkJoinTask<?> job;
    if (task instanceof ForkJoinTask<?>) // avoid re-wrap
        job = (ForkJoinTask<?>) task;
    else
        job = new ForkJoinTask.RunnableExecuteAction(task);
    externalPush(job);
}

You're too quick :)

Oh I only realized I could use this trick because I saw the code and the re-wrap comment led me to this optimization =)

2.2.5 is sync'd to central. Thanks again!

@ben-manes cheers!

I think I found another problem with scheduling, so I'm going to release again. I'm not sure if it is benign or not, as it is very weird. Basically the ForkJoinTask#exec() needs to return false to more reliably re-run in a timely fashion.

Released. Central has been really slow to sync lately so might take a over an hour. I hope this is the last one for the weekend.

@ben-manes they are having some cache invalidation issues.

Thanks for informing me about the issue!

Seems to be the theme of the weekend then :)
2.2.6 is up on Central.

I will investigate a little further soon but it isn't something silly like the fact we never invoke complete when returning true? eg.
    final class PerformCleanupTask extends ForkJoinTask<Void> implements Runnable {
        private static final long serialVersionUID = 1L;
        private final Void NIL = null;

        @Override
        public Void getRawResult() {
            return null;
        }

        @Override
        public void setRawResult(Void v) {}

        @Override
        public boolean exec() {
            try {
                run();
            } catch (Throwable t) {
                logger.log(Level.SEVERE, "Exception thrown when performing the maintenance task", t);
            }
            complete(NIL);
            return true;
        }

Nope, just invokes the raw result method.
public void complete(V value) {
915           try {
916               setRawResult(value);
917           } catch (Throwable rex) {
918               setExceptionalCompletion(rex);
919               return;
920           }
921           setCompletion(NORMAL);
922       }

doExec also calls setCompletion if done.
    final int doExec() {
        int s; boolean completed;
        if ((s = status) >= 0) {
            try {
                completed = exec();
            } catch (Throwable rex) {
                return setExceptionalCompletion(rex);
            }
            if (completed)
                s = setCompletion(NORMAL);
        }
        return s;
    }

It appeared to be a race where that reinitialize was sometimes stampeded by FJP. Then the task was viewed as done and the pool ignored the re-submission. The cache's state machine viewed the task as executing so it wouldn't re-submit and draining wasn't occurring. So this seemed to be a race between the task's internal finalization (our code complete) and a new submission.

It looks like with this change we don't need reinitialize anymore because the task isn't complete, so it is resubmitted. This appears to be primarily intended for forking tasks so that the same instance is reused, but in our case it serves for repeated task exection.

Worth a code comment above the return false explaining why, wdyt?

Good idea. There are too many subtle details that are hard to document and test for.

@JoeHegarty squashed :+1:

Thanks guys!


Cursor Hover

Description
I think it would be nice when hovering cursor over the icons/images under Batch section, the selected images get enlarged to signify what is being selected and even add a hyperlink when clicking on images.
Currently Images are not interactive at all, this feature may enhance Website experience and batch selection on HomePage.

I have started working on this new feature.
Can I get assigned to this task?
So my work does not get interfered by others.
Thank You
@Ask-Sarthak-Goel @Vishal-raj-1 @ManishBisht777 @ManishBisht7

@monisanees270798 sure, you can work this issue

I have completed the task but I am facing a problem somewhere else Batches -> index.mdx.
I cannot seem to understand why changing just tracks.tsx (with "use client" and some additional code) is causing an error in Getting Started -> Introduction.
Any help would be helpful

Hey @monisanees270798 I am not able to understand what do you want to convey? Could you please share screenshot of code which you are not able to understand

Hey @Vishal-raj-1
Thank you for reaching out. I identified the problem and managed to correct it (It was because I used "use client;" within tracks.tsx) causing client/server image declaration). Instead I performed a CSS to resolve it.
I will now commit and push it to my branch   (https://github.com/monisanees270798/Official-Website/tree/cursorHover)
Please review it and let me know if I can procees with the PR.
Many thanks
@Vishal-raj-1

@monisanees270798 You have to raise the PR. So that I review it

Please review and let me know your feedback on my PR.
Thank you.

@Vishal-raj-1 @ManishBisht777 please mark this as closed


Apparent deadlock in tcmalloc

Hello,
A program I'm profiling with gperftools consistently hangs. It seems to be a deadlock issue:

Running it with GDB, pausing once in a while, the stack trace is always the same (see below).

Stack trace of the only thread:
Thread 1 "brep-multitool" received signal SIGINT, Interrupt.
0x00007ffff7a149a6 in sys_futex (v3=0, a2=0x0, t=0x7fffffff6c00, v=<optimized out>, o=128,
    a=0x7ffff7dcf468 <tcmalloc::Static::pageheap_lock_>) at ./src/base/linux_syscall_support.h:2485
2485	in ./src/base/linux_syscall_support.h
(gdb) bt
#0  0x00007ffff7a149a6 in sys_futex (v3=0, a2=0x0, t=0x7fffffff6c00, v=<optimized out>, o=128,
    a=0x7ffff7dcf468 <tcmalloc::Static::pageheap_lock_>) at ./src/base/linux_syscall_support.h:2485
#1  base::internal::SpinLockDelay (w=0x7ffff7dcf468 <tcmalloc::Static::pageheap_lock_>, value=2, loop=<optimized out>)
    at ./src/base/spinlock_linux-inl.h:84
#2  0x00007ffff7a14886 in SpinLock::SlowLock (this=this@entry=0x7ffff7dcf468 <tcmalloc::Static::pageheap_lock_>)
    at src/base/spinlock.cc:119
#3  0x00007ffff79f4748 in SpinLock::Lock (this=<optimized out>) at src/base/spinlock.h:71
#4  SpinLockHolder::SpinLockHolder (l=<optimized out>, this=<synthetic pointer>) at src/base/spinlock.h:133
#5  (anonymous namespace)::do_malloc_pages (heap=heap@entry=0x1d28c40, size=size@entry=40370216) at src/tcmalloc.cc:1328
#6  0x00007ffff7a1891b in (anonymous namespace)::do_malloc (size=40370216) at src/tcmalloc.cc:1357
#7  tcmalloc::do_allocate_full<tcmalloc::malloc_oom> (size=40370216) at src/tcmalloc.cc:1758
#8  tcmalloc::allocate_full_malloc_oom (size=40370216) at src/tcmalloc.cc:1774
#9  0x00007ffff1cf0c62 in cv::fastMalloc(unsigned long) () from /usr/lib/x86_64-linux-gnu/libopencv_core.so.3.2
#10 0x00007ffff1c8c5c0 in cv::Mat::create(int, int const*, int) () from /usr/lib/x86_64-linux-gnu/libopencv_core.so.3.2
#11 0x00007ffff1ca82d3 in cv::_OutputArray::create(cv::Size_<int>, int, int, bool, int) const ()
   from /usr/lib/x86_64-linux-gnu/libopencv_core.so.3.2
#12 0x00007ffff1d90fdf in cv::Mat::convertTo(cv::_OutputArray const&, int, double, double) const ()
   from /usr/lib/x86_64-linux-gnu/libopencv_core.so.3.2
#13 0x0000000000a3a58c in normalize_image_with_opencv (cad_image=..., cad_mask=...,
    target_mean=std::vector of length 8, capacity 8 = {...}, target_std=std::vector of length 8, capacity 8 = {...},
    target_sigma=std::vector of length 7, capacity 7 = {...}) at ../microCalcs/processedImgNormalization/processedImgNormalization.cc:530
#14 0x0000000000a3eacf in normalize_processed_image (image=..., mask=..., dist=<EMAIL_ADDRESS>
    opencv=opencv@entry=true, cuda=cuda@entry=false, verbose=false)
    at ../microCalcs/processedImgNormalization/processedImgNormalization.cc:292
#15 0x00000000009a24c1 in brep::workers::PixelAnalyzer::make_processed_normalized_image (this=this@entry=0x38d6010)
    at ../MAMMOCAD/PIPELINE/pixelanalyzer.cc:317
#16 0x00000000009be2d8 in brep::workers::PixelAnalyzer::operator() (this=0x38d6010) at ../MAMMOCAD/PIPELINE/pixelanalyzer.cc:141
#17 0x00000000008e399b in std::function<void ()>::operator()() const (this=<optimized out>) at /usr/include/c++/6/functional:2136
#18 libconquer::task::operator() (this=<optimized out>)
    at /local/buildbot/tmp/gcc-release-training-build-1334-install/include/libconquer/task.hpp:46
#19 libconquer::queue::run (this=0x2fb5a90) at /local/buildbot/tmp/gcc-release-training-build-1334-install/include/libconquer/queue.hpp:180
#20 0x00000000008d1497 in libconquer::run_until_empty (q=std::shared_ptr (count 91553669, weak -49727257) 0x7fffffff9d50)
    at /local/buildbot/tmp/gcc-release-training-build-1334-install/include/libconquer/queue.hpp:472
#21 brep::Elephant::process (this=this@entry=0x7fffffffd2f0, processing_configuration_=..., featuredir="", regdir="")
    at ../MAMMOCAD/PIPELINE/elephant.cc:97
#22 0x00000000006dee84 in run_elephant (stop_on_exceptions=<optimized out>, verbose=<optimized out>, receiptfile=..., regdir=...,
    featuredir=..., images=std::vector of length 0, capacity 0, studies=std::vector of length 1, capacity 1 = {...},
    cases=std::vector of length 0, capacity 0, archives=std::vector of length 0, capacity 0, jobs=<optimized out>,
    processing_configuration=...) at ../MAMMOCAD/PIPELINE/multitool.cc:696
#23 main (argc=8, argv=0x7fffffffe8f8) at ../MAMMOCAD/PIPELINE/multitool.cc:629

This is on Ubuntu 16.04, using gperftools 2.7, built from source with --enable-frame-pointers. I am using LD_PRELOAD=/usr/lib/libtcmalloc_and_profiler.so.4 CPUPROFILE=cpu.prof to link the profiler dynamically.
Any hints on how to debug this further?

If I link only with libprofiler and not with libtcmalloc, the execution eventually finishes. The profiler then reports that 90% of execution is in __libc_fork.

met same problem, any help?

If  this is deadlock we'll need backtraces from all threads. Or at least other thread that has page heap lock and stuck somewhere else.

Thanks @alk. For me it is not relevant anymore because this was 5 years ago. But @yqynju, as you reported it a bit more recently, maybe you have some additional info?


Provide a dynamic return type for the service container.

When fetching services, the return type is not correct. There should be a service map like https://github.com/phpstan/phpstan-symfony. This consumes the dumped container XML. We don't have that luxury.
Might need to resolve #4  first so we don't load every single services.yml

Since this issue has a commit with a similar-sounding commit message: could we close this issue?

Thanks, @Fonata. I can close.


Llama-2-70b-chat-hf model fails to pass the 125k passkey test

Hi, great work!
I have been conducting passkey tests on several models. The TinyLlama-1.1B-Chat-v1.0(2k) model successfully passed the 20k and, after fine-tuning, the 125k tests with a 60% accuracy rate. However, the Llama-2-70b-chat-hf(4k) model only achieved 40% accuracy in a 50k context and 0% in a 125k context.
I have been using the following script:
from chunkllama_attn_replace import replace_with_chunkllama

replace_with_chunkllama(pretraining_length=4096)
model_path = '/model/Llama-2-70b-chat-hf'
tokenizer_path = model_path
model = LlamaForCausalLM.from_pretrained(model_path, torch_dtype=torch.float16, device_map="auto")
tokenizer = LlamaTokenizer.from_pretrained(tokenizer_path)
model.eval()

def gen_prompt(all_count):
    prompt = "There is an important info hidden inside a lot of irrelevant text. Find it and memorize it. I will quiz you about the important information later.\n"
    n = random.randint(0, all_count)
    prompt += 'The grass is green. The sky is blue. The sun is yellow. Here we go. There and back again.\n' * n
    pass_key = random.randint(10000, 99999)
    prompt += f'The pass key is {pass_key}. Remember it. {pass_key} is the pass key.\n'
    prompt += 'The grass is green. The sky is blue. The sun is yellow. Here we go. There and back again.\n' * (all_count-n)
    prompt += 'What is the pass key? The pass key is '
    return prompt, pass_key

for i in range(10):
    prompt, target = gen_prompt(5000)
    prompt_postfix = "What is the pass key? The pass key is "
    input_ids = tokenizer(prompt, return_tensors="pt").input_ids.cuda()
    print("-----------------------------------")
    print(f"#Tokens of Prompt: {input_ids.shape[1]}", end=" ")
    print(f"Passkey target: {target}")

    tokens = model.generate(input_ids, max_new_tokens=6)
    r = tokenizer.decode(tokens[0].tolist()[input_ids.shape[1]:], skip_special_tokens=True)
    answer = f"ChunkLlama:     [{prompt_postfix}{r}]"
    answer = answer.replace("\n", "\\n")
    print(answer)

The results I've been getting are as follows:
#Tokens of Prompt: 125070 Passkey target: 97476
This is a friendly reminder - the current text generation call will exceed the model's predefined maximum length (4096). Depending on the model, you may observe exceptions, performance degradation, or nothing at all.
ChunkLlama:     [What is the pass key? The pass key is 9456. Remember]
-----------------------------------
#Tokens of Prompt: 125070 Passkey target: 96339
ChunkLlama:     [What is the pass key? The pass key is 934212]
-----------------------------------
#Tokens of Prompt: 125070 Passkey target: 10344
ChunkLlama:     [What is the pass key? The pass key is 1234. Remember]
-----------------------------------
#Tokens of Prompt: 125070 Passkey target: 64645
ChunkLlama:     [What is the pass key? The pass key is 654632]
-----------------------------------
#Tokens of Prompt: 125070 Passkey target: 55491
ChunkLlama:     [What is the pass key? The pass key is 54211.]

How can I achieve results consistent with those reported in README?
Thank you.

Hi! Thank you for this issue!
The results reported in README and our paper are measured by perplexity.  However,  Llama-70B-chat should not achieve such poor performance. Based on your code, I suggest following the chat template of Llama2-chat models like [INST] <<sys>> This is a passkey retrieval task<</sys >>long doc What is the pass key? [/INST]  The pass key is  (please refer to run_llama_100k.py). If changing the prompts doesn't work,  please feel free to leave a comment  andI will detailedly check what happens.

Thanks for your response!
I've changed the prompts to
def gen_prompt(all_count):
    B_INST, E_INST = "[INST]", "[/INST]"
    B_SYS, E_SYS = "<<SYS>>\n", "\n<</SYS>>\n\n"
    sys_prompt = "This is a passkey retrieval task.\nThere is an important info hidden inside a lot of irrelevant text. Find it and memorize them. I will quiz you about the important information there."
    n = random.randint(0, all_count)
    content = 'The grass is green. The sky is blue. The sun is yellow. Here we go. There and back again.\n' * n
    pass_key = random.randint(10000, 99999)
    content += f'The pass key is {pass_key}. Remember it. {pass_key} is the pass key.\n'
    content += 'The grass is green. The sky is blue. The sun is yellow. Here we go. There and back again.\n' * (all_count-n)
    question = 'What is the pass key?'
    message = B_INST + B_SYS + sys_prompt + E_SYS + content + f"Question:\n{question}" + E_INST + "Answer:\n"
    return message, pass_key

and got answers like
#Tokens of Prompt: 125100 Passkey target: 95994
This is a friendly reminder - the current text generation call will exceed the model's predefined maximum length (4096). Depending on the model, you may observe exceptions, performance degradation, or nothing at all.
ChunkLlama:     [The key is 942.]
-----------------------------------
#Tokens of Prompt: 125100 Passkey target: 77050
ChunkLlama:     [The key is 750.]
-----------------------------------
#Tokens of Prompt: 125100 Passkey target: 16789
ChunkLlama:     [The key is 788.]
-----------------------------------
#Tokens of Prompt: 125100 Passkey target: 33519
ChunkLlama:     [The key is 31A.]
-----------------------------------
#Tokens of Prompt: 125100 Passkey target: 54180
ChunkLlama:     [The pass is 542.\nThe grass is green. The sky is blue. The]

Let me know if I did anything wrong.

Thank you so much for letting me know! It will  definitely help us improve this work.
I will try to solve this problem on Monday.

Hi I've updated the code in chunkllama_attn_replace.py via adding temperature adaption proposed by Yarn (Line 65). Based on my test, Llama-70B (chat) now can a achieve a 100% acc on the 100k passkey retrieval task.

Thanks for your work. But I get OOM this time.
I'm running on 8xA100.
-----------------------------------
#Tokens of Prompt: 125100 Passkey target: 51703
Traceback (most recent call last):
  File "/data/model/test.py", line 69, in <module>
    tokens = model.generate(input_ids, max_new_tokens=20)
  File "/usr/local/lib/python3.10/dist-packages/torch/utils/_contextlib.py", line 115, in decorate_context
    return func(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/transformers/generation/utils.py", line 1474, in generate
    return self.greedy_search(
  File "/usr/local/lib/python3.10/dist-packages/transformers/generation/utils.py", line 2335, in greedy_search
    outputs = self(
  File "/usr/local/lib/python3.10/dist-packages/torch/nn/modules/module.py", line 1518, in _wrapped_call_impl
    return self._call_impl(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/torch/nn/modules/module.py", line 1527, in _call_impl
    return forward_call(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/accelerate/hooks.py", line 166, in new_forward
    output = module._old_forward(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/transformers/models/llama/modeling_llama.py", line 1183, in forward
    outputs = self.model(
  File "/usr/local/lib/python3.10/dist-packages/torch/nn/modules/module.py", line 1518, in _wrapped_call_impl
    return self._call_impl(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/torch/nn/modules/module.py", line 1527, in _call_impl
    return forward_call(*args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/transformers/models/llama/modeling_llama.py", line 1043, in forward
    attention_mask = _prepare_4d_causal_attention_mask(
  File "/usr/local/lib/python3.10/dist-packages/transformers/modeling_attn_mask_utils.py", line 307, in _prepare_4d_causal_attention_mask
    attention_mask = attn_mask_converter.to_4d(
  File "/usr/local/lib/python3.10/dist-packages/transformers/modeling_attn_mask_utils.py", line 121, in to_4d
    causal_4d_mask = self._make_causal_mask(
  File "/usr/local/lib/python3.10/dist-packages/transformers/modeling_attn_mask_utils.py", line 160, in _make_causal_mask
    mask = mask.to(dtype)
torch.cuda.OutOfMemoryError: CUDA out of memory. Tried to allocate 29.15 GiB. GPU 0 has a total capacty of 95.04 GiB of which 17.10 GiB is free. Process 3318804 has 77.94 GiB memory in use. Of the allocated memory 76.74 GiB is allocated by PyTorch, and 5.12 MiB is reserved by PyTorch but unallocated. If reserved but unallocated memory is large try setting max_split_size_mb to avoid fragmentation.  See documentation for Memory Management and PYTORCH_CUDA_ALLOC_CONF

I have adapted the code to transformers 4.37.  So please remember to  set the attn_implementation parameter.
In this case:  model = LlamaForCausalLM.from_pretrained(model_path, attn_implementation="flash_attention_2", device_map="auto",                                                     trust_remote_code=True, torch_dtype=torch.bfloat16)

It works like a charm!
Thank you for your patience.

Thank you! This issue really helps improve this work. I am willing to answer any further questions.


Bug: Prometheus client causes immediate websocket disconnect

Description
When using the Prometheus integration, websockets always disconnect immediately after opening the connection.
I've done a bit of digging, and the error arises in PrometheusMiddleware._get_default_labels here, where request.method raises an attribute error for websockets. One possible fix that I've tested locally is to change it to:
"method": (
    request.method
    if request.scope["type"] == "http"
    else request.scope["type"]
),

I'd be happy to open a PR for this change or some other fix if someone has a better idea.
URL to code causing the issue
No response
MCVE
from litestar import Litestar, websocket_listener
from litestar.testing import TestClient
from litestar.contrib.prometheus import PrometheusConfig, PrometheusController

config = PrometheusConfig()

@websocket_listener("/")
def websocket_handler(data: str):
    return f"Hello, {data}!"

app_with_prometheus = Litestar(
    route_handlers=[websocket_handler, PrometheusController],
    middleware=[config.middleware]
)

app_without_prometheus = Litestar(route_handlers=[websocket_handler])

def test_app(app_to_test):
    with TestClient(app=app_to_test) as test_client:
        with test_client.websocket_connect("/") as socket:
            socket.send("World")
            response = socket.receive()
            print(response)

test_app(app_without_prometheus)
test_app(app_with_prometheus)

Steps to reproduce
1. Run the MCVE
2. Observe the traceback

Screenshots
No response
Logs
Traceback (most recent call last):
  File "/Users/erik/test.py", line 27, in <module>
    test_app(app_with_prometheus)
  File "/Users/erik/test.py", line 20, in test_app
    with test_client.websocket_connect("/") as socket:
  File "/Users/erik/Library/Caches/pypoetry/virtualenvs/*****-ocilal_E-py3.11/lib/python3.11/site-packages/litestar/testing/websocket_test_session.py", line 52, in __enter__
    message = self.receive(timeout=self.client.timeout.read)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/erik/Library/Caches/pypoetry/virtualenvs/*****-ocilal_E-py3.11/lib/python3.11/site-packages/litestar/testing/websocket_test_session.py", line 195, in receive
    raise WebSocketDisconnect(
litestar.exceptions.websocket_exceptions.WebSocketDisconnect: KeyError('method')

Litestar Version
2.0.0b2
Platform

[ ] Linux
[X] Mac
[ ] Windows
[ ] Other (Please specify in the description above)

Funding

You can sponsor this specific effort via a Polar.sh pledge below
We receive the pledge once the issue is completed & verified

@JacobCoffee can you try to reproduce this?

"method": request.method if request.scope["type"] == "http" else request.scope["type"], does indeed solve this but I am not certain it is the right place to fix. Investigating...

@erik-hasse would you like to open a PR with your fix?
    def _get_default_labels(self, request: Request[Any, Any, Any]) -> dict[str, str | int | float]:
        """Get default label values from the request.

        Args:
            request: The request object.

        Returns:
            A dictionary of default labels.
        """

        return {
            "method": request.method if request.scope["type"] == "http" else None,
            "type": request.scope["type"],
            "path": request.url.path,
            "status_code": 200,
            "app_name": self._config.app_name,
        }


Truncation of tag values to 256 chars

I have found this library very useful; thanks for creating it!
However, when I use with Jaeger on ElasticSearch, I find the SQL query tag value is truncated to 256 characters when I look at it in the Jaeger UI.
I couldn't find any definitive info on whether this is expected behaviour in Jaeger, however I did find some comments that tags should be things you can filter on, so the full SQL query is not really suitable as a tag value. It could be done as a 'log' rather than a tag.
Posting this mainly for comment; I'm not really sure what I should expect.

Note that the truncation of tags to 256 characters is a jaeger-client-go quirk. You can change the max limit of tags using the MaxTagValueLength config option. So while it may be true that tags aren't the best place to store longer strings, it doesn't seem to bother other clients as much. For example the java client defaults to 1000 characters.


Color visibility of some note

What docs page needs to be fixed?
Not a big problem but the color of the links under "Defining the mapDispatchToProps Function with bindActionCreators" does not seem visible  will reading

Section: Defining the mapDispatchToProps Function with bindActionCreators
Page:Connect: Dispatching Actions with mapDispatchToProps

What is the problem?
The color of the links note under Defining the mapDispatchToProps Function with bindActionCreators
What should be changed to fix the problem?
the purple and grey color don't seem to make it visible while reading i would recommend changing the colors a bit

the contrast ratio between the background and foreground is 8.61, which passes both AA and AAA WCAG standards:

if we change it from primary-darkest back to the default of primary, it drops down to 5.43 which only passes AA:

i do agree that the links stand out less with the current primary-darkest colour though.

Okay, thanks for the quick response. my concern is in dark mode. I don't know if in dark mode this is how the note should look like?


Integration test for dockercfg remover hook

Enables the docker credential cleanup hook by default, and adds an integration test.
Issues
DCOS-5255 Docker credential cleanup hook
DCOS-8955 Docker credential cleanup DC/OS integration test
Checklist

[X] Included a test which will fail if code is reverted but test is not: This is the test for DCOS-5255
[X] Followed relevant code rules Rules for Packages and Systemd

@adam-mesos ,   PR template needs an update to reflect the state.

@nfnt Please update the description appropriately, or tell me what you want me to put there if you can't do it yourself for some reason.

It seems that I cannot update the description at the top. JIRA is DCOS-8955. Automated testing is blocked by DCOS-13902, because the hook module is enabled by a changed cluster config. Given a cluster with changed config, the test should work though, I'll test that today.

Looks like the hook never worked. When creating a custom cluster with dcos_remove_dockercfg_enable == 'true', MESOS_HOOKS won't get set. After setting it manually, the integration test passes. It seems that calculate_mesos_hooks and calculate_use_mesos_hooks in calc.py are somehow wrong, but I don't have a deep understanding how this works in DC/OS, can someone take a look?

Oops! Looks like somebody inserted some dcos-log config files in the middle of what was supposed to be /etc/mesos-slave-common, which means that we're also ignoring MESOS_DOCKER_REGISTRY and MESOS_DOCKER_CONFIG. I can commit a patch on top of this branch to fix it real quick...

This probably worked in 1.8.x, but around Argus3-time, we frantically merged some things that broke it, so that explains why it was so hard to get this thing working again after rebasing.

Added that fix to its own PR https://github.com/dcos/dcos/pull/1359 for new Blocker https://jira.mesosphere.com/browse/DCOS-14473 so we can land it urgently if we need to.

Thanks! All targets are green now, I'll remove the "WIP" label.

... I cannot remove the "WIP" label, because I didn't open the PR. @adam-mesos could you do that?

@mesosphere-mergebot bump-ee

@mesosphere-mergebot bump-ee

@mesosphere-mergebot bump-ee

@mesosphere-mergebot bump-ee

@mesosphere-mergebot bump-ee

@mesosphere-mergebot bump-ee

@mesosphere-mergebot bump-ee

@mesosphere-mergebot bump-ee

@adam-mesos should we try to revive this? :-)

I'd love to see this revived and merged, but I don't think it's been anybody's priority for a while now.

Note that without this test, the dockercfg remover hook (used by at least 1 Mesosphere customer) is completely untested and could break at any time.

Can we close this PR?

This looks important. Maybe worth breathing some more life into and landing @nfnt ?

Please create a new PR to bring this in. If we want this test, we will have to start with the scratch for sure (JIRA, needs, approach etc).. This PR could be helpful as a reference.


Download fails every once in a while

Fix would be to retry a couple times.

Fixed in android_changes branch.


Key-value pairs page

As with units, this adds a brief page explaining the
motivation behind the introduction of "maps", also
known as key-value and/or named-value pairs. More
complete examples of use should be placed with the
code for regular testing.
cc: @hflynn

NB: currently working on the HQL examples.

Looks good except your changes to the What's New page are going to conflict with #1177

Shall I just roll-back and leave whatsnew to you?

Might be easier to merge mine and then merge develop in here and fix it up? I can't add a link to my PR until this new page is merged anyway, so one of us is going to have to do that anyway.

Ok. Excluding. If you want to drive this Monday while I'm out, feel free to close and re-open elsewhere.

Ah ok, I'll fix it up one way or another on Monday.

Merged, fixed, & pushed.

Staged at http://www.openmicroscopy.org/site/support/omero5.1-staging/developers/Model/KeyValuePairs.html.

Can you make the code blocks appear as code instead (add ::)?

@manics : code blocks pushed with a few other fixes.

@mtbc: suggestions pushed. @hflynn: added a pretty techy section on queries. Let me know if it's too dense.

Thank you, this is looking really helpful.

A few nitpicks but nearly there! I think the What's New page would probably read better if you put these new points after 'Version requirements' as the 'Model graph operations' point follows them nicely.
All formats fine except that malformed doc link though.

Pushed.

:+1:


Add Trigger.dev project

🚀 This is a great idea! This PR adds the Trigger.dev project to the list of projects to learn from and adds them to the relevant sections. If you want to add a "Remix" section we could be in there as well since our API/Dashboard is implemented in Remix.

Yes. Add a remix section. That would be great, thanks!

Resolved conflicts and added us to the Remix section 👍


Travis fails due to timeout exceptions

Describe the bug
Travis currently has a chance to fail due to timeout exceptions.
We can observe the problem for around two weeks by now. Restarting the build has a chance to get rid of the issue, but we should investigate why this happens in the first place.
According to my tests, the issue can only be observed inside of Travis. Local Karma-tests are running just fine.
How to Reproduce

Submit a PR

Expected behavior
The timeout exception should not occur
Additional context
Chrome 73.0.3683 (Linux 0.0.0) MotionsMotionSlideComponent should create FAILED
	Error: Timeout - Async callback was not invoked within 5000ms (set by jasmine.DEFAULT_TIMEOUT_INTERVAL)
	    at <Jasmine>
Chrome 73.0.3683 (Linux 0.0.0): Executed 284 of 383 (1 FAILED) (skipped 1) (0 secs / 3 mins 38.061 secs)
Chrome 73.0.3683 (Linux 0.0.0) MotionsMotionSlideComponent should create FAILED
	Error: Timeout - Async callback was not invoked within 5000ms (set by jasmine.DEFAULT_TIMEOUT_INTERVAL)
	    at <Jasmine>

Screenshots

Hopefully resolved by #5047 - keeping open to track further

Should be fixed now.


Should multiple inscriptions on the same sat be possible?

We should approach this issue carefully, since inscriptions cannot be removed.
Ideas:

No. Once a sat has been inscribed it cannot be re-inscribed.
No. Once a sat has been inscribed it cannot be re-inscribed. But, if a transaction tries to inscribe a sat multiple times, those additional inscriptions will still be reference-able  by inscription ID, they just won't be on a particular sat.
Yes. Show inscriptions on same sat with equal prominence.
Yes. Show inscriptions after the first with less prominence, like comments.
Yes, but you can only do so in a single transaction. Further transactions will not inscribe.

I'm leaning towards no. If you want to have a list of things, you can use a collection. And you can already use an inscribed ordinal as a prime ordinal, so if you want things to appear on that ordinals page, then you can just do that.

Yes, it should be possible, because otherwise you need to know all previous inscriptions to know if an inscription is valid, which we want to avoid to keep consensus for features like symbols. #884 covers the issue of defacing inscriptions with other inscriptions.

But we have to consider HTML inscriptions where you want to include a bunch of files in one inscription. The natural way to do this is with multiple inscriptions on the same sat. So maybe allowing multiple inscriptions on the same sat, but not displaying them with equal prominence, perhaps behind a link, is the answer.

Another purpose for multiple inscriptions on a sat might be to collect artist's signatures at a later time. Ie. an artist mints a  limited edition set of inscriptions of their work. You purchase one and then see the artist give a live talk where they offer to "sign" your sat in some way (perhaps by providing you a signed message unique to that sat that you then inscribe on the already inscribed sat.

I have a separate inscription indexer that I'm working on and have used it to produce this table of multiply inscribed sats, with their inscription ids:
<PHONE_NUMBER>28348 |     2 | {b2acef469488bf9ba089a57f0f071f367c8a80d398113df26046e42327920832i0,f7ac52da8c1640239bf909d4058c06716457efd6744b15bb3ad44b966396f2a5i0}
<PHONE_NUMBER>38980 |     2 | {5cc66dc49244cd7f1c0a052af34de9c703eee7d4e06303ee263966d1a7a5c162i0,b0517512fc35e1ab445d317f79827c8523c8c05443274b29cc6d68a8686e90e5i0}
<PHONE_NUMBER>81598 |     2 | {887de5fe22a389d4305cf93e8fe0967f794e96c9bc8c1ae7dedd8ca1f924851fi0,523152810ce88685fbf4442ece6fbd8817bcb6ea82103f69db983344c5fc6704i0}
<PHONE_NUMBER>20672 |     5 | {a0ae52cf8a5adc061f55ae894708c89e1a8d00ba5ba1fdc8cd51ef3bf5aa1f04i0,c303b099b09073f12e99fc9d3481dce59e0b881181ef355fe9ae12e4625fc714i0,568a03b6d52665c190bd28376510d4df64de3702b845df2731b6f7fbbe52d77di0,8c213eaa08f53040eca18e61df3d9ae72291884522c6fbba26a1d17ec4370719i0,de1c3c9539928ceb86564dca6788d115dbd42cb1e48208bdad9838501438ba39i0}
<PHONE_NUMBER>81929 |     2 | {6be88ab6b2227736deb9213a82dc601ab79854193631cdbee398faa23b1f71dfi0,a9141e3e0e90526cc33eaf0dce55c47b2b4301d98f8a51fe2a92e9e277d6428ai0}
<PHONE_NUMBER>08397 |     3 | {7d0d3c5c8c880f69f0d4066b37609636417da85023ff0f4b65da56e8bcff5530i0,140f0711dba52d9d8b7a2f2a180118ebaae6c54b97f7956ddae080b246c2c2a3i0,6449eb2aab8fcba035f02f1dbe1ac5ea54c34ad05e8ffdf684ebc8c35eb14d6ei0}
<PHONE_NUMBER>28944 |     2 | {cb83850b518ac4d86f655ac9714b057da5a0f466e019787eb00d906b0ee300d9i0,51e71e1dcbc0391595870d8e2a1819e6f00d6e46d5b33379dee197a3a20bf404i0}
<PHONE_NUMBER>98213 |     2 | {7153cec0fd76a5eadb1f0f05c5b4e23382a7fa959b54edef02b27422e0a30904i0,aefdc6591cbd62d26984d13493487f136fcf3bc44165cb78956b800301409addi0}
<PHONE_NUMBER>25363 |     3 | {617b02026b7d56c85a41c07ebfd67a1186f0888fd07c59428f11b1b9cdf6a84ci0,de1c1bacf4311067ab547a658afe47dbaf361423e62695bff4d5eac2a4124c9ei0,525dc94711d5e76a83ce7dde6b85706d97ada3205b9f5cc53709edbb4ced76cei0}
<PHONE_NUMBER>54182 |     4 | {3646828db8e2656ce7fe5e468a6cd34b71409ea286b499270e21b1b8e1a16608i0,156a787e50ef1b912830ca1f495fbb2032a159d477844aa84dd63f777bb4db33i0,5ee59cb5f2b88d1aa1dd7ef0f6263a2682412866e8cdb73275fa013429169623i0,414f69e1ce439fdf58dcd2edddcf06eaf26090b9f60a2ac3920d38a555e25cddi0}
<PHONE_NUMBER>43457 |     3 | {b6a8dcbec41a6406c95089c443fb301bf8bf21b5b22d402b42ac6bd2e75c8047i0,e1da773a40fdb26422535d15764fbb34bfcdbc782bcdf9a34dc740e8125ada13i0,1135b067a619323acdcdaff828f495e8d85ea0c3cb5bc9e318959184222b5432i0}
<PHONE_NUMBER>64106 |     2 | {5a59070517e812107d7d71d11b1f5ebce7359710b2c13417a615e2ce3c615e4ai0,10b111dc809638d394a72988c4d1b4b50c01777cc49156ea19557fb30d42755fi0}
<PHONE_NUMBER>87482 |     2 | {d07a8aa92acff133b437ddfb19fc745064dc0ff2425e66e0fb35e75e1ce0600bi0,b90c2edf20c9d944ca8fe5f5973634459c189bb008d1ccf438fbd54173b3fb9fi0}
<PHONE_NUMBER>91423 |     2 | {2ada24450bb5d23c5b91f1ee30a6ce378aaa9d74555476f7505093fd8959a741i0,c0ebbf93aba271b2faad85ea58471a479865b54e79537f69f8739bfa3aceb3d5i0}
<PHONE_NUMBER>00976 |     2 | {584e9e4355c28ff56f463849b52a8ca80b34ea7b5c3b2aa0e61c483b6fe4b419i0,218bef23fab13557c3385d74c05b9158d99120342aad31527c28c764fbcdbc20i0}
<PHONE_NUMBER>73789 |     2 | {d3965032c3a1d6679bc89b160e243f560c0f5daf621e01c2bc6c437f719dc823i0,6673a6f9c307a5448dd470833031fbe92d352e863891e5391e86cd947375bbeai0}
<PHONE_NUMBER>85384 |     2 | {1f769789b2bb6821cd9efd60a9ceb327ff0eaad3f62a106c7e5ac12ca9e9eb62i0,7de170363a823475994025f78d3176d4d2fe068f99bdcbeb8dbe03adaaaaac6ei0}
<PHONE_NUMBER>742579 |     2 | {7c68d5b34c01eb3a6027551817c66fcac3643165c8ce8039811c3dbe62794edei0,2fbc91e6b9e5da25a12e0d2a29eea0cf2c93a9704c87b70f9b826b521fb27befi0}
<PHONE_NUMBER>834189 |     5 | {f67f1c2310427b4063c2dd6e1cd982fc5ad2aa23622c3c7edebeb863e2c2ceb1i0,1e16eb3da4a459c51b8d425eaa7b77ed0a617e5c3eb9444abfc612e75fb7dd1di0,5b92c222e95f398e922fc8b87105fe3c2e303e6800253e7337d3f8196f263155i0,8048b885a8b0c2f9ce87fc7cd8f107e5725c38aec54530e5993ffe367c112c46i0,c91c2bd708c1ba7bd21e0af0415a2d12e65da5f167cbb413ba59b17771d10761i0}
<PHONE_NUMBER>553721 |     3 | {2793c5d0fbafc357a93059bc9750dcf84fae69e5281e72518590ead024409086i0,9bc69b85c1b7d0cbfbe6b04d71cfb2962fbd2ad15d0438cbdf1e6d5afdf2450bi0,bed251324e05a9c57905251ba83f5eff0068ff24f8af28ee3fb6e3317fb9b38bi0}
<PHONE_NUMBER>268131 |     2 | {d39ba38a0766a0734dfb7ee962b6383a690d5b474da1b2ca816edd8332499f91i0,3d14fedb3f3a62df26d4b3c3ed61fb66d61b57d9fadf980a1cc2cd04a3adc5c0i0}
<PHONE_NUMBER>887422 |     2 | {cd3b44f5f67c9c89f3a25a75eec6ffbe1d6fd3940f052e18cdb38c9e2ea1b688i0,773f7c6d2048e0ad2b4392979a72a74a9e6bbfb25cb97d57571e668f68fa316fi0}
<PHONE_NUMBER>243586 |     2 | {adb372bf2279e7f677dff273fa3a74b36c82652fe6e1823418eb7b6df7ee600bi0,307914db62bf00a26a816ca89dae7c3badfa555eec7b8f7b5fee5429402b08abi0}
<PHONE_NUMBER>259426 |     2 | {e747b1a1cd91cdb106ce696508425da6103c72daf94861fbf22c1786adbd5a55i0,18276968a48859382ad7fce8ce794d80191cf0002ad66f9988cb6375b0be59fci0}
<PHONE_NUMBER>311198 |     2 | {50725bc848b585a3d088d6caeea8333203977521cb086ad73ad45ceb8ddf2d3ei0,f217aa3940a32a569183298ca526ab77b684964732d4c832f537e3cef08f7f13i0}
<PHONE_NUMBER>791376 |     2 | {4c23a07605c661fe2638a876535313a23d98991f1e3ee159877b9ddeae3e79b0i0,61cebb693dbe5030d27437ad2252539abd2cb3ff5a12854b9cfe9e49505c7179i0}
<PHONE_NUMBER>497468 |     2 | {27b43e2b684a3771a04abf49e1543c913e200d4a54f56024cccdce33c211a4c2i0,9611ffc39bc30e100f0003528b90e7a086534609f782c5acd91beed15b440500i0}
<PHONE_NUMBER>507549 |     2 | {e0a36bf3addc4d8a6f432ebd8983b3e7f1cff8bf746f01139c50a710ed418004i0,1c8449533387115c6f31216f51bcf24126f13806c06fdb9484d7f45232c1289ai0}
<PHONE_NUMBER>447952 |     4 | {5686a0102c9be41dfb4d20457056887136684043a75af7482d38a6ff085cc00bi0,e5735df21935bdb70144826ff3efc536c2350ac6ac77542429a03dee1e7ce057i0,f660446bc2a98357869eac4d35cd1d1cb8d10a81de64a4b8fe7d965d6efa2708i0,490a64146b15da8e211adb019601f3b9d64655400067ca6863553706ccc6cd51i0}

Thought it might be interesting on this issue and potentially #1455

Please support this 🙏

Binding multiple inscriptions onto the same sat would be desirable for several reasons - from the perspective of a collection owner it would allow us to deliver incentives to owners and ensure the related inscriptions always move together when transferred.


After user change setting permission to None. Check permission PERMISSIONS.IOS.PHOTO_LIBRARY / MEDIA_LIBRARY got UNAVAILABLE not DENIED

Before submitting a new issue

[X] I tested using the latest release of the library, as maybe the bug has been already fixed.
[X] I checked for possible duplicate issues, with possible answers.

Bug summary
I have faced with issue about request permission PERMISSIONS.IOS.PHOTO_LIBRARY and PERMISSIONS.IOS.MEDIA_LIBRARY. Let me explain

When first time using app. The func check(PERMISSIONS.IOS.PHOTO_LIBRARY) got result is UNAVAILABLE. That same with request or IOS.MEDIA_LIBRARY.

After that, if user go to setting and change permission to None. Func check(PERMISSIONS.IOS.PHOTO_LIBRARY) got result is UNAVAILABLE too, that is same with the first time ask. It makes me unable to distinguish between 2 results so I can use the request function to ask again.

Is there any way to help me distinguish them?
Thank you very much,
that image setting about 2 step above,

======
i'm using xcode 15, build on iphone 8 (OS 16.4), iphone 11 prm (OS 17.2.1), iphone 14 prm (OS 17.3.1)
Library version
3.9.3
Environment info
System:
    OS: macOS 14.1
    CPU: (8) arm64 Apple M1
    Memory: 74.91 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
    Yarn: 1.22.21 - ~/Work/ssm-mobile-app-parent/node_modules/.bin/yarn
    npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
    Watchman: 2<IP_ADDRESS> - /opt/homebrew/bin/watchman
  Managers:
    CocoaPods: 1.14.3 - /Users/chubo/.rvm/gems/ruby-2.7.4/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 23.0, iOS 17.0, macOS 14.0, tvOS 17.0, watchOS 10.0
    Android SDK: Not Found
  IDEs:
    Android Studio: 2023.1 AI-231.9392.1.2311.11255304
    Xcode: 15.0/15A240d - /usr/bin/xcodebuild
  Languages:
    Java: 21.0.2 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.1.0 => 18.1.0
    react-native: 0.70.12 => 0.70.12
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to reproduce

…
…

Reproducible sample code
export const checkPermissionAccessAlbum = async (t: any) => {
    const result = await check(Platform.OS == 'android' ? PERMISSIONS.ANDROID.READ_MEDIA_IMAGES : PERMISSIONS.IOS.PHOTO_LIBRARY)

    switch (result) {
        case RESULTS.BLOCKED:
        case RESULTS.DENIED:
            Alert.alert(t('allowPhotoAccess'),
                t('noPermissionAlbum'),
                [
                    { text: t('gotoSetting'), onPress: () => Linking.openSettings() },
                    { text: t('rejected') },
                ]
            )
            return false;
        case RESULTS.UNAVAILABLE:
        case RESULTS.GRANTED:
        case RESULTS.LIMITED:
        default:
            return true
    }
}

First, update the library to the latest version.

Closed, no answers provided (might be solved in the latest version?)


Endow MatrixGate with a type parameter specifying number of qubits it acts on

This should help to create type-stable tailored code for applying a MatrixGate.

...and the actual matrix stored in MatrixGate could be a dense (instead of sparse) matrix.


Ctrl+hover (goto definition preview) causes textDocument/didOpen(/didClose) events

VSCode Version: 1.36.1
OS Version: Windows 10

Steps to Reproduce:

Hover on some symbol while holding Ctrl so the "definition preview" box appears.
Observe textDocument/didOpen sent to the language server. Also textDocument/didClose seems to be sent right after that.

While this doesn't sound like a big ussue, it can cause severe slowdowns if an extension is doing something on document open. For example the Haxe language server requests diagnostics for the opened file, which can be expensive at times.
In practice it's pretty bad, because the events happen on any ctrl+click, even if the user's intention is just quickly go to the definition.
If these events are intended, then perhaps they could contain information about the cause of document open (e.g. "intent": "preview"|"peek"|"edit"), so the language server can decide whether it should do additional work or not.

I'd expect that publishing diagnostics on file open events is a pretty common pattern for language servers, so this would affect a lot more than just Haxe. It's especially silly because the diagnostics aren't even rendered in the preview.

This already happens in the extension host (it is not LSP specific). The reason is to get the text around the destination (e.g. the content rendered in the upper part of the hover).
I like the idea of having an intent so that clients listening to the event can better react to it. But before this can make it's way into LSP we first need to have it in the VS Code API.
@jrieken is there another trick / solution you could think of?

But before this can make it's way into LSP we first need to have it in the VS Code API.

If I understand it correctly, alternatively the extension's LSP client could use that info to decide to NOT send events to the language server, which would fix this particular issue. Because it doesn't seem to make a lot of sense to send the events to the LS for this preview?

@jrieken is there another trick / solution you could think of?

That's the designed behaviour because there is only one way of opening a document. The underlying problems seems to be that diagnostics are driven by open/closed documents, not by visible editors nor by visible tabs (the latter has no API)

The API request to expose the tab model: https://github.com/microsoft/vscode/issues/15178 (which might serve as duplicate here). Generally, I think there are different strategies how validation should happens (be driven by)

project wide - when a file belongs to a project (like tsconfig.json, foo.csproj) and I would expect that then every file for a project is checked
editor wide -  those documents that I see (visible editor, not open document) or better all tabs (#15178)
open documents - tricky as document open/close without the user being able to observe that, e.g there are extensions that open all documents matching a certain criteria (like all css files). also, document usually don't close immediately when editors close

Well this boils down to the definition of "open document", because I wouldn't recognize that preview box as an open document. But then again one can argue that it is indeed an open document... I don't know :)

Regarding the "visible editors" API - to make it useful it has to be in the LSP as well, but does it make sense there conceptually?

Regarding the "visible editors" API - to make it useful it has to be in the LSP as well, but does it make sense there conceptually?

I honestly don't know... If it doesn't, does then exposing the tab model makes sense? Tho the LSP client can abstract that way and call it 'none-project-validation-queue'; it being a list of documents that the user likely wants validation for.

What do you think about adding the "intent" property to the textDocument/didOpen data? That sounds useful and could be a simpler solution for the issue.

could be a simpler solution for the issue.

It unfortunately isn't. For once, there is an API challenge because today the onDidOpenTextDocument-event fires the document, not an event object that can be extended. Then, internally it will be hard to funnel that information all the way across our different layers

I see, well then we'll think of some workarounds, thanks!

Great. I think a pragmatic approach would be to check with vscode.window.visibleTextEditors

Yeah, the main problem is that we gotta communicate that to the language server that is also supposed to work with non-VSCode clients, but I guess the server could behave differently based on init configurations.


Copying a book does not trigger a webhook event

Describe the Bug
When you copy a book that's in a shelf and have a webhook active where one of the events is "book_create", the webhook will never fire.
Steps to Reproduce

Make a webhook with the event "book_create" or "bookshelf_update"
Copy a book in a shelf
Observe the webhook never firing

Expected Behaviour
I expect a webhook to be fired to run my scripts to update book permissions
Screenshots or Additional Context

Browser Details
No response
Exact BookStack Version
22.10.1
PHP Version
7.4
Hosting Environment
Azure App Service Instance

So, this webhook actually just doesn't fire period. I've recreated it and it doesn't even attempt to try any more.

Copying a book should log a book create activity, and therefore call the webhook action of the same name.

Do you see a book-create activity logged in the audit-log/homepage?
Do you have a QUEUE_CONNECTION option defined in your .env file at all?

Copying a book should log a book create activity, and therefore call the webhook action of the same name.
* Do you see a book-create activity logged in the audit-log/homepage?

* Do you have a `QUEUE_CONNECTION` option defined in your `.env` file at all?

I do have a QUEUE_CONNECTION in my env:
CACHE_DRIVER=database
SESSION_DRIVER=database
QUEUE_CONNECTION=database
I do see an audit log for the book create but I still don't see the webhook being fired. Even my other webhook for page updates isn't firing.

I do have a QUEUE_CONNECTION in my env: QUEUE_CONNECTION=database

Yeah, that would do it. With this set to database emails and webhooks will be queued onto the database. A separate service is used to pull these jobs off the queue and action them.
The "Queue Worker Process" part of our webhook docs details the required setup. If that separate service is not running then jobs (webhook and email sends) will just sit on standby. Comment/remove the QUEUE_CONNECTION to go back to normal non-async webhook sending or ensure the queue worker is active.
Just be aware that, if you re-run the queue worker right now it'll send everything queued up (All non-sent emails/webhooks) almost at once. I think you can clear existing queued jobs via php artisan queue:clear.

I do have a QUEUE_CONNECTION in my env: QUEUE_CONNECTION=database

Yeah, that would do it. With this set to database emails and webhooks will be queued onto the database. A separate service is used to pull these jobs off the queue and action them.
The "Queue Worker Process" part of our webhook docs details the required setup. If that separate service is not running then jobs (webhook and email sends) will just sit on standby. Comment/remove the QUEUE_CONNECTION to go back to normal non-async webhook sending or ensure the queue worker is active.
Just be aware that, if you re-run the queue worker right now it'll send everything queued up (All non-sent emails/webhooks) almost at once. I think you can clear existing queued jobs via php artisan queue:clear.

I think this is the 3rd time you have pointed that I just can't read.
I remember doing what I was trying to do...then I got pulled away and I completely forgot what I did. So I never did the part where I actually make it work.
Thank you...again!


Add more languageids

Add missing languageIds.
Add support for .jsonc.
Add support for .db.
Fix sql file extension icon and remove duplicate.

P.S. _file_sql icon can be deleted since it represented MySQL logo, but sql is not only related with MySQL.

Thank you man.